-
Notifications
You must be signed in to change notification settings - Fork 4
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
Define spec for development environment field #15
Comments
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as outdated.
This comment was marked as outdated.
This comment has been minimized.
This comment has been minimized.
But in this case, the user is the maintainer. This whole configuration block only applies when it’s the developer of the package/project. |
How does I'd love to hear from yarn or pnpm if and how they'd plan on handling the failure state where the user has indicated they want to download the correct version. How do you pick the version? Do you spawn it yourself? Why not fail and let the user remediate? |
Good point. As you can see the lines between "installing as module" and "interacting as app" are so blurry that even I missed that. |
If a URL is provided, we’re just downloading that exact URL. The version constraint wouldn’t apply. If we want to allow |
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This is a great start. I think it's important that it not just be limited to the runtime and the package manager - |
Tagging relevant people to review the draft spec: @Ethan-Arrowood @arcanis @zkochan @aduh95 |
I am not very good at reading specs like this. What would it look like in implementation to limit the local dev environment to a given os or cpu for example? From what I see it doesn't look like that field is sufficiently decoupled from "named" which doesn't make sense in that context. |
|
This proposal covers way more than package manager and would not be under that field name (although it is not mentioned above). I think that should have always been the goal, so that is why keeping the field "experimental" in node core was important. I agree we would want to smooth the path. But I think that is up to the folks who implemented the field in their tools. Maybe here we would just discuss goals for ways to make that transition? |
The first step should be figuring out (such that they can be clearly documented and conveyed) the problems we're solving, and the goals and non goals - and then to sketch out a solution that seems viable to all stakeholders. At that point, those interested in the migration path from the experimental |
Cross-posting from Slack: I’ve attempted to read all of the various GitHub and Twitter threads that have led us here. I think there is a lot going on in general, and I don’t have a clear, concise reply just yet. For now, all I will say is that I encourage all invested parties to join the monthly call on March 5th. We rarely have much to discuss regarding the group’s active work streams, so we can reasonably spend the entire hour discussing the corepack issue.
|
I added those fields because you had mentioned them on the other thread. I'm not sure what the use case would be; run npm on Windows and Bun on macOS? I can just remove them if you can't think of a use case either. We can always add fields later. |
I think that os/cpu etc are in the category of "general engines worth explicitly giving a range for", and altho "runtime" and "package manager" are reasonable to call special, the schema needs a mostly free-for-all dumping ground too (like |
Yeah, remember we're defining a spec for all development runtime constraints, not just package manager. Package manager is a subset of constraints. Node version, os, cpu, and libc are other constraints. |
Why would you allow specifying multiple different package managers? I assume there are still people in this group that are strongly against lockfiles. But at least don't motivate devs to use different package managers on a project. It will make everyone's life a lot harder. |
My guess is this is done to remain flexible, not make a recommendation that folks should. That said, I could think of a few cases where maybe you want to use
I don't think that would be a driving part of the decision is it? I assume you are meaning that if a package declared multiple dev package managers they would then have to resolve the lockfile differences? I think at this layer of abstraction (a spec for the package.json) we would not want to exclude package managers from agreeing on a shared lockfile format (in fact we might want to encourage that 😎) so I think it is reasonable to keep. |
How about changing “hash” into two fields: “algorithm” and “digest”? |
That’s fine, what would be an example? This? "download": {
"url": "https://registry.npmjs.org/@yarnpkg/cli-dist/-/cli-dist-3.2.3.tgz",
"algorithm": "sha224",
"digest": "16a0797d1710d1fb7ec40ab5c3801b68370a612a9b66ba117ad9924b"
} |
|
What's the "hash" currently? In npm lockfiles it's the output of |
For folks who need it (me) here is what EDIT: I should have added, this is an implementation of the subresource integrity spec used in browsers as well: https://w3c.github.io/webappsec-subresource-integrity/ So with that in mind, I would say that seems like it could be a good spec to just directly here. |
At runtime, a package would support 1 or N runtimes, and pnpm would use whatever's first in the list, I suppose, but it also shouldn't matter since any runtime in
|
Yes, you are right, you can ignore my points about the runtime part. However, the part about dependencies could be still valid. |
I am highly suspicious that long term this is a really bad decision. I would happy to be proven wrong, but my spidy senses tell me this is asking for problems. |
If each project is a subdirectory in the monorepo, then each subdirectory could have its own package.json with engines/devEngines to define a different Node.js version. |
Two projects use the same dependency (inside node_modules). One project has devEngines set to Node.js 18 and the other one to Node.js 20. What Node.js version should be used to run the dependency's postinstall script? We don't build it separately for each project. |
devEngines should not have any impact on anything unless it's in the current project - so things inside node_modules only use |
@zkochan I love that That said, I'd encourage you - & others - to make any runtime values/definitions equivalent to dependency specs if not rely on the existing dependency schemas (ie. Although Notably, I could see this effort/work pushing projects like
I'd argue that a CLI should be a self-contained/standalone executable & if there is any runtime requirement it should be defined as a dep &/or peer-dep. That said, I totally understand this has not been the standard practice in our ecosystem (ie. CLI tools have come to rely on
I would be mindful not to optimize any future APIs/tooling for a legacy feature like install scripts. Instead of leaning further into them, I think we'd benefit much more from working together on something like package distributions where the expectation is that a package is consumable without the need for arbitrary script execution. If there's a use case that isn't handled by the linked RFC, I'd love to flush it out with you so we have a consistent experience for end-users going forward (binding/linking native addons via the distribution config is definitely one area that requires more thought).
It's extremely rare that you run a dependency's script definitions outside of lifecycle scripts & even then your script/task should be agnostic of any executables not defined as dependencies (see my previous comment on defining runtimes as deps/peer-deps).
This sounds a lot like This question reaffirms my belief that we should support multiple runtimes & runtime versions at the project level (just like we do with all other dependencies). The fact that
What's a "current project"? What prevents a tool from reading I harp on this point time & time again because I've seen, firsthand, how development-specific configuration becomes indistinguishable from "production"/distribution configuration devoid of context. The most egregious of these is |
@darcyclarke the current project is the package.json whose dependency list is being installed. nothing prevents a tool from reading devEngines inside node_modules except that it'd be a categorically incorrect and nonsensical thing to do. If you cd into a node_modules/foo or node_modules/@foo/bar dir, then that becomes the current project, which is how npm treats it as well. Tools always must do "project root resolution", as npm does, and without that there's no way to meaningfully run the majority of useful commands. |
Depending on how a tool has implemented monorepos/workspaces, file or linked-deps (ie. if the tool allows for those to live in |
Certainly if a tool has created its own concept of layout, then it'll have to figure out how devEngines works with that. The intention/purpose/design of it, though, is to only apply when a project is being developed, never when it's being consumed/installed. |
Hi folks! This thread is getting quite long and very hard to follow. During the monthly call, we discussed the following steps:
Based on the new PRs from npm, I think the timing for this couldn't be better! Would anyone like to volunteer to do step 1? Otherwise, I will do my best to draft something and then ask folks to review it. Thank you for the great work everyone! |
I can open a PR with the top post as a file. |
Not sure if this has been touched on. This does have some interesting consequences for runtimes which do |
I think that'd be up to the runtime providing emulation - if it's confident it's the same, then it'd be ok to pretend it's the same, and if it's not exactly the same, it wouldn't count. I think one runtime actually emulating another is a rare enough scenario that we don't have to plan for it much. |
npx ls-engines --dev --save Actually, 'devEngine' should be used instead of 'engine', but we have to wait for: openjs-foundation/package-metadata-interoperability-collab-space#15 Signed-off-by: Sebastian Davids <[email protected]>
This PR adds a check for `devEngines` in the current projects `package.json` as defined in the spec here: openjs-foundation/package-metadata-interoperability-collab-space#15 This PR utilizes a `checkDevEngines` function defined within `npm-install-checks` open here: npm/npm-install-checks#116 The goal of this pr is to have a check for specific npm commands `install`, and `run` consult the `devEngines` property before execution and check if the current system / environment. For `npm ` the runtime will always be `node` and the `packageManager` will always be `npm`, if a project is defined as not those two envs and it's required we'll throw. > Note the current `engines` property is checked when you install your dependencies. Each packages `engines` are checked with your environment. However, `devEngines` operates on commands for maintainers of a package, service, project when install and run commands are executed and is meant to enforce / guide maintainers to all be using the same engine / env and or versions.
|
Awesome! Within the next week, I'll try to make a PR to npm to add a way to make it useful in a CI matrix. |
With #27 merged, lets continue and start new discussions regarding this field through separate issues or PRs to that document. Everyone is welcome to contribute details to that document as well if there are certain discussions/decisions you'd like to be documented. Thank you all for the collaboration here 🚀 together we are making JS development better and better |
openjs-foundation/package-metadata-interoperability-collab-space#15 Signed-off-by: Sebastian Davids <[email protected]>
This is a draft spec for nodejs/node#51888 (comment), to define the runtime and package manager for developing a project or package (not consuming/installing it as a dependency in another project). We could use the name
devEngines
and it would be a new top-level field defined with this schema:The
os
,cpu
,libc
,runtime
, andpackageManager
sub-fields could either be an object or an array of objects, if the user wants to define multiple acceptable OSes, CPUs, C compilers, runtimes or package managers. The first acceptable option would be used, andonFail
would be triggered for the last defined option if none validate. If unspecified,onFail
defaults toerror
for the non-array notation; or it defaults toerror
for the last element in the array andignore
for all prior elements, for the array notation. Validation would check the name and version ranges.The
name
field would be a string, corresponding to different sources depending on the parent field:os
: NPM docs forengines.os
.cpu
: NPM docs forengines.cpu
.libc
:glibc
ormusl
; see NPM docs forconfig.libc
.runtime
: WinterCG Runtime Keys.packageManager
. . . I don’t know, we could define the list as part of the spec, or perhaps it would need to correspond to an npm registry package name. Suggestions welcome.The
version
field syntax would match that defined forengines.node
, so something like">= 16.0.0 < 22"
or">= 20"
. If unspecified, any version matches.The
onFail
field defines what should happen if validation fails:ignore
: nothing.warn
: print something and continue.error
: print something and exit.download
: remediate the validation failure by downloading the requested tool/version.In the event of
onFail: 'download'
, it would be the responsibility of the tool to determine what and how to download, perhaps by looking in the tool’s associated lockfile for a specific version and integrity hash. It could also be supported on a case-by-case basis, like perhaps Yarn and pnpm could support downloading a satisfactory version while npm would error.Typical example:
“Uses every possible field” example:
Some potential future expansions of this spec that have been discussed are:
runtime
andpackageManager
might take shorthand string values defining the desired name or name with version/version range.The text was updated successfully, but these errors were encountered: