Skip to content
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

Bug: Make disabling the no-unpublished-x rules in private packages opt-in #297

Closed
1 task done
IchordeDionysos opened this issue Jun 13, 2024 · 7 comments · Fixed by #298
Closed
1 task done

Bug: Make disabling the no-unpublished-x rules in private packages opt-in #297

IchordeDionysos opened this issue Jun 13, 2024 · 7 comments · Fixed by #298
Labels

Comments

@IchordeDionysos
Copy link

Environment

Node version: 20.11.0
npm version: 10.2.4
ESLint version: 8.57.0
eslint-plugin-n version: 15.7.0
Operating System: macOS

What rule do you want to report?

no-unpublished-import, no-unpublished-require

Link to Minimal Reproducible Example

https://github.com/eslint-community/eslint-plugin-n/tree/master/tests/fixtures/no-unpublished/private-package

What did you expect to happen?

Make disabling no-unpublished-x optional in private packages.

Participation

  • I am willing to submit a pull request for this issue.

Additional comments

#57 (comment)

@IchordeDionysos
Copy link
Author

IchordeDionysos commented Jun 13, 2024

I'm not sure what the best approach would be to tackle this bug.

I see two option how to achieve this:

  1. Add a flag "allowPrivate" (or similar) that disables the flag in a private project
  2. Revert the PR feat: Disable the no-unpublished-x rules in private packages #57 and ask users to disable the rule when they need to via the .eslintrc.json file (e.g. when they are in a private package)

Both options are better than what we currently have, where there is NO option to enable the rules in a private package.

@IchordeDionysos
Copy link
Author

@aladdin-add @voxpelli @bmish happy to get some insights from your side how to approach this.

@aladdin-add
Copy link

I'm fine with opt-in, however it's a breaking change (we won't be releasing it anytime soon).

I suggest:

  1. adding an optionallowPrivate(defaults to true).
  2. ❓in the next major, change the defaults to false.

@aladdin-add
Copy link

@scagood @voxpelli thoughts on this?

@IchordeDionysos
Copy link
Author

@aladdin-add I've opened a PR with ignorePrivate for now :)
Happy to discuss the naming or other approaches to it.

@scagood
Copy link

scagood commented Jun 13, 2024

I'm on the fence about this one, one part of me thinks that we should accept this option.
As knowing whether I can install only the production dependences is a great thing (eg making a docker image).

This is the main sentiment in:

Feedback on this change:
Not all projects are packages in a typical sense, and some projects are services deployed to a server or built as a website.


The thing I am not sure about is if we want to add the complexity to the rule, or if we want to split the file check out of the dependencies check

@scagood
Copy link

scagood commented Jun 13, 2024

Just looking at #298 the change looks simple enough :)

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

Successfully merging a pull request may close this issue.

3 participants