Skip to content

fix(schematics): add support for eslint.cjs files #4137

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from

Conversation

JeromeWirth
Copy link
Contributor

Closes #3583

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

[X] Bugfix
[ ] Feature
[ ] Code style update (formatting, local variables)
[ ] Refactoring (no functional changes, no api changes)
[ ] Build related changes
[ ] CI related changes
[ ] Documentation content changes
[ ] Other... Please describe:

What is the current behavior?

Closes #3583

What is the new behavior?

Does this PR introduce a breaking change?

[ ] Yes
[X] No

Other information

Copy link

netlify bot commented Nov 18, 2023

Deploy Preview for ngrx-io ready!

Built without sensitive environment variables

Name Link
🔨 Latest commit 37a1873
🔍 Latest deploy log https://app.netlify.com/sites/ngrx-io/deploys/6559225f957ace0008ce630c
😎 Deploy Preview https://deploy-preview-4137--ngrx-io.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link
Member

@timdeschryver timdeschryver left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you add a test case for this please?
This change detects js config files, but I assume that the implementation (https://github.com/ngrx/platform/blob/main/modules/eslint-plugin/schematics/ng-add/index.ts#L22) needs to be updated as well.

@timdeschryver timdeschryver added the Needs Cleanup Review changes needed label Dec 10, 2023
@NathanLaing
Copy link
Contributor

If this stays stale I'm happy to pick it up and write some tests.

Out of interest are other .eslint.? supported?
The default is .json so it makes sense we handle that.
The piece I am missing is is there somewhere we handle the others (i.e. .js, .ts, .mjs) or are they not yet supported?

@JeromeWirth
Copy link
Contributor Author

Sorry for the late reply. This is the point where im currently stuck. For now only .json is allowed and i'm not sure how to transform it into a valid .cjs file.

Sine im bit busy currently feel free to pick it up.

@timdeschryver
Copy link
Member

We're currently only support the .json format (and we have tests for this).
Because this PR is/will become stale I'm closing this.
Feel free to create a new PR with the needed changes to support other formats, e.g. .js.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Cleanup Review changes needed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ng schematic does not recognise .eslintrc.cjs files
3 participants