-
Notifications
You must be signed in to change notification settings - Fork 2
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. |
I see no harm in having additional, optional properties. We should specify other things like if non-specified properties are allowed or not. This will be important when we get a JSON schema established. |
I understand what those fields mean in the The original proposal suggested
If we disallow additional properties, wouldn’t that mean that we can’t extend this spec in the future? |
yes, plenty of repos simply can't be developed on windows, or only can be developed on windows, for various reasons. |
Just that. Development only works in that given environment. Feel free to leave that out of the spec, npm will probably be including it in our own implementation as a non-standard extension though. Again, given the problem we're solving is "engines etc, but for local dev" leaving those out doesn't seem appropriate. I think this is close enough that we can start implementation, hopefully a PR can shape up by next week, probably in npm-install-checks |
The goal of this is to avoid exactly this comment. I don't think it is valuable to leave it out if you are going to implement it and there is clearly a use case.
Did you see the conversation we were having in slack? I don't believe we have addressed everything. |
No sorry @wesleytodd I didn't see it. I still think a draft PR will help since a lot of the effort on our side is going to be implementation specific. The object itself can change format easily. It will be a draft PR with the purpose of helping npm shake out exactly where this code needs to live and what its api is in npm itself. |
Conversations in other channels are fine, to a point. Not everyone is in every slack though so if folks can at least summarize important discussions here that'd be swell. |
I am hoping to avoid extra work for y'all honestly.
Yes, strong agree. The problem has been these threads have been too long to keep up with and add to that everyone frustration with this continuing discussion I didn't want to brainstorm in the thread. We can brain storm in another thread or issue on GH which is fine. But this thread is not the one either for that. Honestly on a VC is the right place because my things were questions and thoughts to work through. If you had npm rfc calls I would discuss there 😉. Without those, tell me where you want me to go? |
Thanks for the feedback @wesleytodd. We'll hold off on implementation then. |
What’s the use case for this, though? It makes sense for |
@GeoffreyBooth many of my packages' dev-time run-scripts don't work on windows. They use env vars and not cross-env, or they use |
This is VERY common and today we don't have a way for folks to indicate it and fail, users of windows just have to know they may not be able to work on projects easily. I do think this is an important use case to accomidate for. |
Okay, that makes sense, I can add it. What about also having it as a field within |
I've never seen that specific limitation, only a broader one for OS, etc in general. ETA: The reasons for folks needing os/libc/cpu are usually because the packages being developed are FOR those endpoints. The node version and package manager is irrelevant there. |
engines field is for package consumption openjs-foundation/package-metadata-interoperability-collab-space#15 Signed-off-by: Sebastian Davids <sdavids@gmx.de>
Any updates here boys? |
Sorry for the radio silence. There was a bit of a miscommunication about whether this was ready for npm to implement, coupled w/ some other things in April that tore our focus away. Unless I hear otherwise this is the spec npm is going to work towards implementing,
The package.json attribute npm will look for is going to be cc @zkochan |
what's the default of |
Version is optional to allow folks to define the package manager in general, not a specific version. Functionally it's the same as defaulting it to "*". |
I thought we had agreed not to do the parts other than |
I think npm is free to implement whatever parts of the spec it wants to support; they said they have no plans to support |
|
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
,runtime
, andpackageManager
sub-fields could either be an object or an array of objects, if the user wants to define multiple acceptable OSes, CPUs, 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
.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.The
download
field would apply only foronFail: 'download'
. I assume no one would support download foros
,cpu
orruntime
. 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. The optionalalgorithm
anddigest
fields would allow for verifying a download, per the https://w3c.github.io/webappsec-subresource-integrity spec.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: