Workflow disruption

[Timer]

Between 8:15 PM - 9:15 PM (EST) newly created applications could not be started with npm run start.

Failed to compile.

Error in ./src/index.js
Module build failed: TypeError: /Users/joe/Desktop/testing/src/index.js: Cannot read property 'scope' of undefined
  @ multi main

One of our dependencies released a backwards incompatible change in a minor release.
We typically try to avoid this by pinning our package versions, but we currently have no way of ensuring deterministic builds.

Few users were affected, but situations like these raise questions about how we can prevent this from happening in the future.

We want to ensure we keep our users' trust in our reliability, and I think this deserves some discussion about what can be done.

---

Is there anything that prevents us from shipping releases with npm-shrinkwrap.json and yarn.lock files (does yarn try use dependents lock files?)?

Should we start bundling dependencies again? There seems to be hard feelings about them: #1068. Are they better in NPM 4? Can we push to make them better?

Or should we live with that accidents happen? 🤷‍♀️

[Conrad2134]

Yarn only uses the top-level yarn.lock - but that should be enough, right? The lockfile includes direct and transitive dependencies. If you package a version that works, it'll work on the client until they run an upgrade.

[Timer]

That might work if we ship it as part of our template, which seems a bit finicky. Maybe we could detect Yarn on the system and copy it from react-scripts.

[Conrad2134]

Keeping a lockfile up to date would be a manual process, right? Each release someone would have to regenerate an up-to-date lockfile if it were being shipped with the template.

[Timer]

We'd probably just push the responsibility of publishing onto our CI server which would generate a lockfile and run tests against it.

[wtgtybhertgeghgtwtg]

Wouldn't that still break when the user runs yarn upgrade or installs a dependency?

[gaearon]

bundledDependencies likely won't get better because they are a fringe feature. They cause other problems, are buggy, and Yarn doesn't support them either.

Shrinkwrap only works on npm but not Yarn.

I think we should just teach people about setting up Yarn and using a lockfile. That solves all problems. We can include this in the User Guide (e.g. “Ensuring Deterministic Builds”).

[Conrad2134]

@wtgtybhertgeghgtwtg - yes and yes, but that's on the user. yarn add shouldn't have much of an impact compared to yarn upgrade (although it could, someone better versed in the ways of Yarn can speak to that).

[Timer]

@wtgtybhertgeghgtwtg I don't believe yarn add upgrades anything unintentionally, but the benefit here is that they can rollback their yarn.lock file (if source controlled) and have a working project instead of manually trying to diagnose and fix errors.

[Timer]

@gaearon adding docs will go a far way, but it still doesn't solve fresh installs unless if we add the yarn.lock to the template

[gaearon]

but it still doesn't solve fresh installs unless if we add the yarn.lock to the template

If user doesn't have Yarn then lockfile wouldn't be useful anyway.
If user has Yarn then yarn.lock will get created automatically in the project folder (I just tried it).

[Timer]

The lock file generated on install wont be of use to them if the registry has a bad published version.
The case you described enables rollbacks for upgrades and unintended breaks when switching machines, but not the initial install.

[gaearon]

Ah, I get it now. Maybe we could bundle last known good yarn.lock for Yarn users?

[Timer]

That's what I was thinking, we can detect if they have yarn and include a yarn.lock file. Should we do the same for npm-shrinkwrap.json?

[gaearon]

Shrinkwraps are a pain to deal with so I don't want to create one by default.

[Timer]

It's odd, npm advises to ship npm-shrinkwrap.json, and Yarn advises to not ship yarn.lock (but doesn't give a reason).

[Timer]

Could we have create-react-app delete the shrinkwrap after initial install? NPM doesn't upgrade sub-dependencies (unlike Yarn) so our pinned versions + initial install with shrinkwrap should be safe.

edit: or even package.json has a postinstall to rm the file / remove it from package.json

[gaearon]

It doesn't make sense to ship yarn.lock for libraries because only top-level yarn.lock is respected (and this is by design). But in our case we are creating a top-level yarn.lock.

[Timer]

Ah, that's right -- that makes more sense. But yes, we'd be making the top-level yarn.lock (which isn't "shipping" it). 😄

[gaearon]

Could we have create-react-app delete the shrinkwrap after initial install?

Then it wouldn't have any effect because the next person to clone the project (e.g. teammate) will use the regular resolution mechanism. So we can't ship a shrinkwrap unless we actually ask people to use it (which I'm not recommending).

[Timer]

Mmm yeah you're right, the next teammate would potentially have a problem then. Sounds like a yarn-only solution for now if we go through with it. 👍 We can wait to see if anyone else provides some feedback.

[gaearon]

Want to send a PR for Yarn-only solution? We can also leave it for contributors if we explain in more detail how it's supposed to work.

[Timer]

I probably wont have time to work on this until the weekend. We could add the help-wanted label for now and I can remove it if no one starts come the weekend (or later)?

I'd be more than happy to answer any questions for someone who'd like to do this.

[Conrad2134]

Would that get rid of the yarn add react react-dom then? You'd basically copy over yarn.lock (which gets generated / committed from CI after a successful build) and run one yarn install?

There'd have to be a CI specific process then - CI does like it does now, but for a normal user it would just run yarn install with the lock file.

I'm happy to take a stab at it if I can wrap my head around what all needs done :)

[gaearon]

Would that get rid of the yarn add react react-dom then? You'd basically copy over yarn.lock (which gets generated / committed from CI after a successful build) and run one yarn install?

Yes. We might want to do it after #1253 which also groups react and react-dom installation together with react-scripts for npm. Can you coordinate with @n3tr on this? I think we could land #1253 first, but maybe it makes more sense to start with it and then add the Yarn thing on top in a single PR.

[Conrad2134]

Per our quick discussion on #1253 - I'll wait until that lands to start the PR for this, if that works.

[Timer]

Closing this as I believe it's pretty inactionable by us.
npm@5 now uses a lock file by default, which should help alleviate this.