-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
Competing lockfiles create poor UX #5654
Comments
It could be a good use case for the |
Round tripping From the This would allow users of both tools to seamlessly switch back and forth without leaving the user's project in a confusing state (where files appear to be from both). |
I'm a bit concerned with this, because it means that neither This issue also raises the question of interoperability between Yarn and npm in general, originally discussed in another thread - I feel like trying to be too interoperable might deserve both of us, since users will then have the expectation that everything will work exactly the same way on both project, which I feel is a dangerous assumption (one obvious example is that workspaces will silently break if someone run npm on a workspace-powered project). @yarnpkg/core, thoughts? |
I like @iarna’s idea to make both tools migrate from one lock file to
another on install automatically.
Makes sense to delete the imported lockfile after the migration to avoid
both package managers competition in one project.
Both tools could print warnings and maybe reqire user prompt to proceed.
I also agree with Mael’s point that achieving 100% compatibility will lock
us from exploring new features, I think we should treat it as a one-off
migration path, that could be a rather small feature in install.js on our
side.
…On Wed, Apr 11, 2018 at 9:49 PM Maël Nison ***@***.***> wrote:
I'm a bit concerned with this, because it means that neither
package-lock.json nor yarn.lock will be able to add metadata to the files
(Yarn currently doesn't, but why not), removing some freedom to our formats
in the process.
This issue also raises the question of interoperability between Yarn and
npm in general, originally discussed in another thread - I feel like trying
to be *too* interoperable might deserve both of us, since users will then
have the expectation that everything will work exactly the same way on both
project, which I feel is a dangerous assumption (one obvious example is
that workspaces will silently break if someone run npm on a
workspace-powered project).
@yarnpkg/core <https://github.com/orgs/yarnpkg/teams/core>, thoughts?
—
You are receiving this because you are on a team that was mentioned.
Reply to this email directly, view it on GitHub
<#5654 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/ACBdWI9jnLJeFqH8v2T-AB74sQO1PMIjks5tntzrgaJpZM4TQ5-B>
.
|
Pinging @imsnif since he has expressed interest in implementing the "Convert the other lockfile" solution a few weeks ago. He may even have some working code by now. |
Thanks for the ping! So yes, I'm actually in the midst of working on exactly this. I was planning on writing up a yarn RFC and pinging all stake holders next week. So briefly: I've done some work on converting yarn.lock <==> package-lock.json. There are some losses in the process, but very little of them are logical. In my eyes, this is totally acceptable if we're talking about the 'one time conversion' scenario mentioned above. If we'd like to discuss this further, I can go into more details. I currently have some rudimentary code that does this: https://github.com/imsnif/synp What I want to do once that is done is start talking about how (and if?) we'd like to implement this exactly in both Would love to hear everyone's thoughts on this. |
I don't think it will be that common. I might be wrong, but I feel like the more common use case will be projects using Yarn, and one of the developer copies/pastes the command from a README to add a dependency, using In this context, I'm still not convinced it's a good thing to lose data and change the package layout silently by trying to do the right thing - they will probably not even notice it until things break (we could have a confirmation prompt, but I'm expecting people that copy/paste those commands to also blindly confirm install prompts). Worse, it could work on their own machine, but break on their collegues' ones once they switch back to Yarn. |
@arcanis - I think all the scenarios and edge-cases are very much worth discussing. In the scenario you mentioned I very much agree, but I think there are other scenarios where there should be a clear bias toward one lockfile. I see this feature as mainly being used in CI environments, where I think it can shine (as OP mentioned). In the very least though, I think both tools should be aware of the other tool's lockfile? Leaving the specifics of when they decide to make the conversion up to them. What do you think? |
Do you have an example? CI systems are supposed to act in a very deterministic way, accidentally commiting a converted lockfile is something that should be caught at worse at PR-time imo, CI is too late for this.
Maybe. Using the In my mind, converting the lockfiles on the fly also isn't very scalable. For example, we've been only talking about the |
Also note that if I understood correctly, npm is currently working on a CI-specific very light package manager, which might not work very well with things requiring heavy business logic such as a lockfile converter. |
Regarding CI: When I previously worked in a mixed npm/yarn environment, we would essentially do this manually with git histories. It was a painstaking process and I'm sure we were/are not alone in this. As for scalability: This is obviously a little ahead of the tool's capabilities right now, but I do not see implementing this as a big issue. |
(as for npm's new ci tool - I see what you mean, but I think this is a little ahead of the current discussion?) |
Yep, I'd be totally for adding this conversion logic into the |
As of |
Oh, and you also need #5042 to be shipped before this is true, as well, since npm doesn't have the |
@arcanis - fantastic! So would a PR with:
@zkat - that's great about |
If by "currently working on" you mean "have shipped" then yes. =) As you suggest, currently loading yarn.locks is beyond its scope (as it doesn't know how to layout package trees) but if we all end up with a library that can do that layout I'm not opposed to having it support loading yarn.locks. (Ideally this would be a library shared between it and Yarn.) Obviously for its case it shouldn't remove the existing lock file. It's purely read-only as far as package metadata goes.
I'm concerned that just having warnings when the an unsupported kind of lock file exists won't help with the problems raised by @jmorrell that Heroku is having? |
A warning would be an improvement, but an error with good messaging would be ideal for users imo. I can only speak for myself, but I have several projects, some that use yarn and some that use npm, and I often find myself typing the wrong one for that project. A quick error means that I can immediately use the one I should have from the beginning. If it's a warning, it should be big and obvious. IME users are pretty trained to expect to ignore the output of their package manager as long as it worked. I've reached out to some people who have more experience with beginning developers who will hopefully chime in on what that experience looks like to someone who's just getting started.
I agree. Ideally by the time it arrives at the build / CI service the application is not in an indeterminate state. The only way I would feel comfortable not failing the build in this case was if there was an accepted way for the user to indicate their preference in |
It would help them by warning the users that they're doing something they probably don't want to before they actually push on Heroku. I'm not sure how much it would actually change the numbers, but that's something we can just check in a few weeks and see if we need to go further. And since the required changes are relatively small, that might be a good first iteration. |
Honest question for those proposing a warning: Is there a situation where running |
Migrating from npm to Yarn, or from Yarn to npm, comes to mind. I'd prefer to avoid adding friction when one wants to try out other package managers. One thing to also keep in mind is that warnings and errors are not mutually exclusive, or at least not with the Another advantage of the engine field is that it would allow you to know for sure which version of the package manager has to be used. I guess it's configurable somewhere, but with this system it would be part of the regular workflow. |
Is there an We support specifying versions in the Ex:
There are a couple concerns that I have with using this to determine which package manager to use:
Ex: "I cloned the example hello-world project, downloaded Node from https://nodejs.org, and it didn't work" All that said, I would love, love, love if the tools enforced strict versions and recorded it in |
AFAIK Yarn enforces this field strictly unless you pass a flag to disable that.
This is quite dangerous, especially when using Yarn since it only guarantees consistency across the same major versions of Yarn.
How about Yarn (and also npm, if they like the idea) adding a |
Awesome 😍 Between this and your work on |
I thought we'd wait for @jmorrell to give us some stats of how this affects the problem on Heroku so that we can decide if this is enough or we'd like to change something (warning/error, etc.) wdyt? EDIT: afaik the warning has not been released yet, so we still have some time to wait. |
How so? Volume of traffic at npm tells you nothing about whether or not the project is open source. a more accurate assessment would be to count which github projects have 0,1 or 2 of (yarn.lock, package-lock.json). personally i have never seen an open-source project (of any appreciable size) on github that has a yarn.lock file and no package-lock.json (except the obvious). IMO if you're going to keep separate lock files, then BOTH package managers should detect the other's lockfile and ERROR (possibly with an override flag). |
All - I'd kindly ask that we stay on topic and take any non-directly-related comments offline (eg. our Discord channel). There are a lot of people subscribed to this thread and in fairness, I feel the npm<>yarn discussion does not belong here. Thanks! |
I havent read the whole tread, but IHMO user should be at least notified that there is an ambiguity: if npm sees a yarn.lock file then npm should print something like: if yarn sees package-lock.json file then yarn should print something like: And as suggested above, a way to "prefer" package manager (in package.json). |
I observed a regression caused to dependencies updates, build is file but you can't use UI to add things (browser complains on module.export = App) Probably caused by babel update from beta49 to rc2 (TBC). Project prefers yarn over npm, but npm install could be used by developers or scripts. Note that, Adding a (potentially unaligned) lock file, could create ambiguity but IMHO it's better to support both than only yarn. Related links: https://stackoverflow.com/questions/44552348/should-i-commit-yarn-lock-and-package-lock-json-files yarnpkg/yarn#5654 (comment) Change-Id: I9489e365b6d3a70102a7d2ae1e44feb7aeb28c06 Signed-off-by: Philippe Coval <[email protected]>
I observed a regression caused to dependencies updates, build is file but you can't use UI to add things (browser complains on module.export = App) Probably caused by babel update from beta49 to rc2 (TBC). Project prefers yarn over npm, but npm install could be used by developers or scripts. Note that, Adding a (potentially unaligned) lock file, could create ambiguity but IMHO it's better to support both than only yarn. Related links: https://stackoverflow.com/questions/44552348/should-i-commit-yarn-lock-and-package-lock-json-files yarnpkg/yarn#5654 (comment) Change-Id: I9489e365b6d3a70102a7d2ae1e44feb7aeb28c06 Signed-off-by: Philippe Coval <[email protected]>
This was not clear to me at all. Up until now I thought it was a developer-local choice, and multiple developers could coexist on a single repo using whichever one they liked, while it being a little more convenient to use only one consistently. The express/hapi etc examples are good though and make it clear that that's not a choice. This should have more visibility.
I think this is the best solution, iff it's implemented in all package managers. The package managers then MUST 100% refuse to proceed if they're called on the wrong project, just as you would expect an error if you required redux but then tried calling mobx functions on it. They should issue an error, not a warning, and inform the user how to proceed with the correct package manager. |
If you want npm to consider this I encourage you to open a discussion in the appropriate venue, but I'll say that I don't like this direction. I would like to see convergence between package managers not divergence. |
I would recommend against some new "package-manager" field, instead recommending the existing Both the yarn and npm docs mention (in passing) usage of https://yarnpkg.com/en/docs/package-json#engines- Even if neither npm or yarn check |
Agreed. Either convergence, or refusal. but the current ambiguous, erroneous middle-ground is harmful. I still think that just throwing an error in the presence of the others' lockfile would require the least friction for existing projects. |
Thank you, I've just signed up. There's a recent thread in this direction: https://npm.community/t/npm-install-should-warn-about-the-presence-of-yarn-lock-files/1822 My vote is for erroring out full-stop instead of just warning, so not sure if I should append to that thread of start a new one.
Same here. Though they are incompatible right now, so the divergence has already happened and is already causing friction to many users and projects. Whatever convergence comes out of further talks amongst the teams is going to take time. When it does arrive, the proposed errors can be lifted and users can start switching and mixing package managers without a cinch. Adding the errors now avoids further confusion and doesn't stop efforts in the direction of convergence.
That sounds reasonable. I'm not attached to any particular method of detecting the presence of an incompatible package manager. To add illustration, my use case in particular is gatsbyjs. The situation is the exact same as reported by @gaearon here:
To avoid this problem, even though gatsby uses yarn, it also adds a |
@imsnif Thanks for the email reminder! Sorry for missing the update here. Here is a graph of the number of apps that experienced this particular failure each week in 2018. (I've redacted the y scale, but this represents 100's of developers and shows the trend)
|
@jmorrell - this is really great! I think (on the yarn side) this issue can be considered resolved. Do you agree? |
i still think the advice of removing IMO if a |
Thanks again for your input @Spongman. I feel this has all been well discussed in the (very long) thread above us and I don't think it makes sense to restart it again. I think we all understand your position. Thanks for participating. Unless @jmorrell tells me otherwise in the next week or so, I'm going to consider this done and close this issue. |
@imsnif Taking a different track and looking at failures from September so far (Sep 1 - Sep 20), this is still the number 1 reason that Node builds on Heroku fail. It happens twice as much as the next most common known failure case: package.json being invalid JSON. Once again redacting exact numbers, here is how it stacks up to other issues. I haven't seen any movement from npm on this, so I'll create a PR adding a warning there too and see it gets accepted.
I don't think this has been resolved for users, and the user experience still isn't great. But I can see how the numbers change once npm also gives feedback to a user hitting this case. I'll leave it up to you whether you want to close this or make further changes. I'm happy to open a new issue in a few months if this is still a big problem for users. |
@jmorrell - seeing as this issue is attracting a lot of attention and the conversation keeps restarting and even repeating itself, I think it would be better to close it now with the information we have. If, with npm providing a warning as well, we still see an issue, I'd definitely be willing to start the error discussion again (or look into other solutions). Please, do ping me personally in that case. Thanks for bringing this up, contributing and making all these changes happen! I personally feel yarn is better after this issue. |
Having both `yarn.lock` and `package-lock.json` (i.e. using both Yarn and NPM) could [easily](yarnpkg/yarn#5654) [be](yarnpkg/yarn#3614) [problematic](https://stackoverflow.com/questions/44552348/should-i-commit-yarn-lock-and-package-lock-json-files). In short, it creates a risk of out-of-sync where one author installs with NPM and forgot to update yarn.lock so another author, when using yarn, won't have that dependency. In this PR I propose to drop the use of yarn in favor of NPM since a search in the codebase yields more NPM usages (especially in the doc and in package.json) than Yarn (only in Docker file to install). If others have strong reasons for using Yarn over NPM then feel free to yield them. I don't have strong personal reason for one over another :D
NB: I'm creating this issue in the yarn repo, but this is actually a shared issue between yarn and npm.
With the release of npm 5 back in May, the Node ecosystem now has two lockfile-based package managers. This was overall a win for users, and it has been good to see competition in this space.
However now that there are two competing lockfile formats this can create a new problem for users, especially beginning developers.
When npm 5 came out, Heroku added a new failure if an application was submitted with both npm and yarn lockfiles. In the past few months this has become the most common reason Node builds fail on Heroku and these failures account for ~10-12% of all Node build failures on the platform. Thousands of developers hit this every month.
It's surprisingly easy to end up with both in your repository. Even as an experienced developer I've found myself running the wrong tool for a specific project and having to catch myself before commiting. For an inexperienced developer building their first server / react / angular project who might not understand what a package manager, lockfile, or git repo is, this can be highly confusing.
Neither tool will give a warning or error if the other lockfile exists:
This is likely especially true for Yarn users where most of the documentation on the web instructs users to
npm install
. Users who copy + paste commands from docs or Stack Overflow are likely to end up here.I've spoken to @zkat and @iarna at npm and @arcanis on yarn, and all agreed that this is an issue that should be addressed, but there was not full agreement on how. Ideally this issue prompts discussion and both tools can agree on how we can help users here.
I've compiled a list of potential mitigations that have been suggested to me:
Potential solutions
Do nothing
Is there a technical reason a user might want two lockfiles? In this case, how can external tools determine which package manager should be used for that application?
Error if the other lockfile exists
Yarn could print an error and exit if
package-lock.json
exists and vice-versa.Pros:
Cons:
Convert the other lockfile
Yarn could read
package-lock.json
, convert it intoyarn.lock
, and removepackage-lock.json
. npm could do the opposite.Pros:
Cons:
Delete the other's lockfile
Just remove the other lockfile and create a new one
Pros:
Cons:
Run the other tool for the user
If yarn sees a
package-lock.json
but not ayarn.lock
it could log a warning and callnpm install
for the userPros:
Cons:
Add config to
package.json
to indicateAdd a field in
package.json
to indicate which package manager a project should usePros:
Cons:
Other?
Maybe I'm missing something that would work better
The text was updated successfully, but these errors were encountered: