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

feat: when strict checking is off, treat like transparent #197

Merged
merged 7 commits into from Oct 24, 2022

Conversation

kenrick95
Copy link
Contributor

@kenrick95 kenrick95 commented Oct 20, 2022

Fixes #195

Relevant to #167 and #157


Edit: final changes:

  • (Breaking change): COREPACK_ENABLE_STRICT=0 now will run as if it is "transparent", that is:
    • If a user is using the package manager specified in the current project, it will use the version specified by the project's packageManager field.
    • If the user is using other package manager different from the one specified for the current project, it will use the system-wide package manager version.
  • Moves the existing behavior of COREPACK_ENABLE_STRICT=0 to COREPACK_ENABLE_PROJECT_SPEC=0

@kenrick95 kenrick95 force-pushed the kenrick/set-no-strict-as-transparent branch 2 times, most recently from c3b73f7 to 7c53577 Compare October 20, 2022 14:11
Copy link
Contributor

@aduh95 aduh95 left a comment

Choose a reason for hiding this comment

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

Would it make sense to add another value to COREPACK_ENABLE_STRICT to keep a way to disable package.json parsing? Maybe COREPACK_ENABLE_STRICT=2 or something? It doesn't have to block this, as it seems the implementation covers the more frequent use case, but I feel like Corepack should give a way to skip all interactions with package.json.

@kenrick95
Copy link
Contributor Author

kenrick95 commented Oct 20, 2022

Would it make sense to add another value to COREPACK_ENABLE_STRICT to keep a way to disable package.json parsing? Maybe COREPACK_ENABLE_STRICT=2 or something? It doesn't have to block this, as it seems the implementation covers the more frequent use case, but I feel like Corepack should give a way to skip all interactions with package.json.

Yeah I think that can be brought up in a separate issue. Although on this matter, my view is that, that behavior removes Corepack's main feature of choosing project-defined package manager, and that makes Corepack yet another npm/yarn/pnpm version manager 😅

@kenrick95 kenrick95 force-pushed the kenrick/set-no-strict-as-transparent branch from 7c53577 to bc08ee3 Compare October 20, 2022 14:31
@aduh95
Copy link
Contributor

aduh95 commented Oct 20, 2022

Would it make sense to add another value to COREPACK_ENABLE_STRICT to keep a way to disable package.json parsing? Maybe COREPACK_ENABLE_STRICT=2 or something? It doesn't have to block this, as it seems the implementation covers the more frequent use case, but I feel like Corepack should give a way to skip all interactions with package.json.

Yeah I think that can be brought up in a separate issue. Although on this matter, my view is that, that behavior removes Corepack's main feature of choosing project-defined package manager, and that makes Corepack yet another npm/yarn/pnpm version manager 😅

I can see it being useful as a debug feature, not for production – while I could see why the behavior implemented in this PR would be useful in production, it's two different use cases IMHO.

@kenrick95
Copy link
Contributor Author

Would it make sense to add another value to COREPACK_ENABLE_STRICT to keep a way to disable package.json parsing? Maybe COREPACK_ENABLE_STRICT=2 or something? It doesn't have to block this, as it seems the implementation covers the more frequent use case, but I feel like Corepack should give a way to skip all interactions with package.json.

Yeah I think that can be brought up in a separate issue. Although on this matter, my view is that, that behavior removes Corepack's main feature of choosing project-defined package manager, and that makes Corepack yet another npm/yarn/pnpm version manager 😅

I can see it being useful as a debug feature, not for production – while I could see why the behavior implemented in this PR would be useful in production, it's two different use cases IMHO.

I see. Though I feel it would be better as a separate flag, like COREPACK_IGNORE_PROJECT_SPEC? COREPACK_ENABLE_STRICT=2 gives the impression of Corepack being extra strict, but in reality, it is not 😅

@arcanis
Copy link
Contributor

arcanis commented Oct 20, 2022

I tend to agree, if the feature was still there it'd make more sense as a separate flag imo (COREPACK_ENABLE_PROJECT_SPEC=0, maybe, by consistency with the other flags?)

@aduh95 aduh95 closed this Oct 20, 2022
@aduh95 aduh95 reopened this Oct 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Use the package manager version specified by the project, but still allow other package manager invocations
4 participants