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.

Sign up for GitHub

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: download the latest version instead of a pinned one #134

Merged
merged 7 commits into from
Aug 19, 2022
Merged

feat: download the latest version instead of a pinned one #134

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
88f6807
feat: download the latest version instead of a pinned one
aduh95 Jun 24, 2022
6c9e041
add `COREPACK_NO_LOOKUP` env flag
aduh95 Jun 26, 2022
0668fb0
Use KGR to cache the version
aduh95 Jun 27, 2022
c33fa1d
don't hardcode the registry
aduh95 Jun 28, 2022
12a91f9
Merge branch 'main'
aduh95 Jul 23, 2022
4ce5e2c
`COREPACK_NO_LOOKUP` -> `COREPACK_DEFAULT_TO_LATEST`
aduh95 Jul 23, 2022
9563bcd
Merge branch 'main' into dynamic-default-version
aduh95 Aug 19, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,8 @@ This command will retrieve the given package manager from the specified archive

- `COREPACK_ENABLE_NETWORK` can be set to `0` to prevent Corepack from accessing the network (in which case you'll be responsible for hydrating the package manager versions that will be required for the projects you'll run, using `corepack hydrate`).

- `COREPACK_NO_LOOKUP` can be set in order to instruct Corepack not to lookup on the remote registry for the latest version of the selected package manager.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We should probably find a better name for this env variable, I don't think one could infer what it's doing from the name only. Does anyone has suggestions?

Copy link
Member

Choose a reason for hiding this comment

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

I'm trying to understand the difference between COREPACK_ENABLE_NETWORK=0 and COREPACK_NO_LOOKUP=1? In theory I would expect no network to behave the same since corepack ships with Known Good Versions but I must be overlooking a use case here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

When COREPACK_NO_LOOKUP is set, Corepack will use the version defined in config.json instead of asking the npm registry for the most up-to-date version. COREPACK_ENABLE_NETWORK can disable all access to the network, both for finding what's the most up-to-date version number and to download the tarball.

Copy link
Member

Choose a reason for hiding this comment

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

Is config.json available to configure by the user of corepack or is it just internals?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's only internals, bundled in the corepack executable.

Copy link
Member

Choose a reason for hiding this comment

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

How about COREPACK_IGNORE_LATEST?

Copy link
Contributor Author

@aduh95 aduh95 Jul 23, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

I went with COREPACK_DEFAULT_TO_LATEST which is true by default, and is set to 0 to use the value in config.json instead.

Copy link
Member

@styfle styfle Jul 24, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Not sure about using “default” as a verb here.

How about COREPACK_ENABLE_FETCH_LATEST=0

Copy link
Contributor Author

@aduh95 aduh95 Jul 24, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

I'd like the naming to make clear that it's only relevant if there's no local clue on what version to use. ENABLE_FETCH_LATEST could mean that the latest is always downloaded, or at least that's how I read it.

Not sure about using “default” as a verb here.

I'm not a native english speaker, so genuine question: is it incorrect or something to use "default" as a verb?

Copy link
Member

Choose a reason for hiding this comment

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

It’s not incorrect, just that “default” can be a verb or a noun. It’s probably fine 👍


- `COREPACK_HOME` can be set in order to define where Corepack should install the package managers. By default it is set to `$HOME/.node/corepack`.

- `COREPACK_ROOT` has no functional impact on Corepack itself; it's automatically being set in your environment by Corepack when it shells out to the underlying package managers, so that they can feature-detect its presence (useful for commands like `yarn init`).
Expand Down
19 changes: 11 additions & 8 deletions sources/Engine.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import defaultConfig from '../config.js

import * as corepackUtils from './corepackUtils';
import * as folderUtils from './folderUtils';
import {fetchAsJson} from './httpUtils';
import * as semverUtils from './semverUtils';
import {Config, Descriptor, Locator} from './types';
import {SupportedPackageManagers, SupportedPackageManagerSet} from './types';
Expand Down Expand Up @@ -69,17 +70,19 @@ export class Engine {
// Ignore errors; too bad
}

if (typeof lastKnownGood !== `object` || lastKnownGood === null)
return definition.default;

if (!Object.prototype.hasOwnProperty.call(lastKnownGood, packageManager))
return definition.default;
if (typeof lastKnownGood === `object` && lastKnownGood !== null &&
Object.prototype.hasOwnProperty.call(lastKnownGood, packageManager)) {
const override = (lastKnownGood as any)[packageManager];
if (typeof override === `string`) {
return override;
}
}

const override = (lastKnownGood as any)[packageManager];
if (typeof override !== `string`)
if (process.env.COREPACK_NO_LOOKUP)
return definition.default;

return override;
const latest = await fetchAsJson(`https://registry.npmjs.org/${packageManager}`);
return latest[`dist-tags`].latest;
aduh95 marked this conversation as resolved.
Show resolved Hide resolved
}

async activatePackageManager(locator: Locator) {
Expand Down
1 change: 1 addition & 0 deletions tests/main.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import {runCli} from './_runCli';

beforeEach(async () => {
process.env.COREPACK_HOME = npath.fromPortablePath(await xfs.mktempPromise());
process.env.COREPACK_NO_LOOKUP = `1`;
});

const testedPackageManagers: Array<[string, string]> = [
Expand Down