feat: add COREPACK_ENABLE_STRICT env variable
#167
Conversation
| manager versions that will be required for the projects you'll run, using | ||
| `corepack hydrate`). | ||
|
|
||
| - `COREPACK_ENABLE_STRICT` can be set to `0` to prevent Corepack from checking |
There was a problem hiding this comment.
It's a bit weird that the name is COREPACK_ENABLE_STRICT but the default is true. You only use this environment variable when you want to disable strict checking.
There was a problem hiding this comment.
It follows the same pattern as COREPACK_ENABLE_NETWORK. Do you have another name suggestion?
There was a problem hiding this comment.
I prefer env vars and booleans alike to always be named "enable" regardless of default value so you don't have to think about double negatives when disabling the "disable" variable.
This allows you to change the default value in the future without renaming the env var which is really nice if the consumer needs to set the env var and doesn't know the version of corepack that will be used.
TLDR: keep it as _ENABLE_
There was a problem hiding this comment.
Separately tho, it's always weird and confusing when a boolean option has different behavior between "absent" and "false".
There was a problem hiding this comment.
I don't have another naming suggestion given that this follows an existing pattern. I'm not trying to block or delay this with my comment.
| @@ -44,6 +44,8 @@ export async function findProjectSpec(initialCwd: string, locator: Locator, {tra | |||
| // A locator is a valid descriptor (but not the other way around) | |||
| const fallbackLocator = {name: locator.name, range: locator.reference}; | |||
|
|
|||
| if (process.env.COREPACK_ENABLE_STRICT === `0`) return fallbackLocator; | |||
There was a problem hiding this comment.
Nit: might not be needed in this PR, but it'd make sense to move the "parsing" logic in a function so we could be sure the same one is used for enableStrict and enableNetwork
Fixes: #157