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.

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

PIN YOUR DEPENDENCIES (Or how I broke CRA for everyone today) #11906

Open
shellscape opened this issue Jan 11, 2022 · 9 comments
Open

PIN YOUR DEPENDENCIES (Or how I broke CRA for everyone today) #11906

shellscape opened this issue Jan 11, 2022 · 9 comments
Labels
dependencies Pull requests that update a dependency file

Comments

@shellscape
Copy link

Is your proposal related to a problem?

Sure is. I'm the maintainer of webpack-manifest-plugin and I bricked create-react-app for a few minutes this morning. Turns out that npm had one of those very rare npm pack hiccups and a version snuck out to the registry without any dist code.

Describe the solution you'd like

Pin your dang dependencies, people! This project is big enough and used by enough people (especially the legions of newbies who don't know how to diagnose or triage dependency issues) that this project should not be using transitive dependencies for critical pieces of library architecture. This

"webpack-manifest-plugin": "^4.0.2",
should not be transitive, it should be pinned.

CRA won't start up if webpack-manifest-plugin is hosed. And it was. So it was effectively bricked.

Describe alternatives you've considered

Pray to the Flying Spaghetti Monster (didn't work)

Additional context

Given the issues that we've seen recently, and most recently with colors.js bricking the world, this seems wise to resolve. I'd bet there are a bunch more critical deps that won't allow CRA to start up should something be awry as well.

@eschwartz
Copy link

Thank you @shellscape for sharing what happened so quickly here 🙏

A committed package-lock.json should prevent previously created projects from breaking. But creating a new project with npx create-react-app will still result in a broken app, even if you pin your CRA version (eg. npx create-react-app@5.0.0).

So a pinned version of CRA does produce consistent output.

CRA could bake a package-lock.json into each release so that npx create-react-app@5.0.1 would always result in the exact same set of files.

@shellscape
Copy link
Author

CRA could bake a package-lock.json into each release so that npx create-react-app@5.0.1 would always result in the exact same set of files.

It's something, but that won't help folks who use pnpm or yarn without explicitly importing the lockfile.

Ideally, at the very least CRA should pin any critical dependencies in package.json at the time of publish. Given the audience size, it might not be a bad idea to do that to all dependencies in package.json at publish.

@Josh-Cena
Copy link

FWIW, won't deprecating this version work? Or even contacting NPM to remove it, if it's within 24 hours and you have good reasons to?

@jjavierdguezas
Copy link

jjavierdguezas commented Jan 12, 2022

yep, classic cra issue #7709
😩

UPDATE
just now I am seeing an error: Loading PostCSS "postcss-preset-env" plugin failed: Unexpected token '.' and PostCSS Preset Env package have been updated just an hour ago...

image

csstools/postcss-plugins#153

no code changes, just another build and boom 💥! so annoying

please, cra team fix the dependencies!!

@petetnt petetnt added dependencies Pull requests that update a dependency file and removed issue: proposal needs triage labels Jan 13, 2022
@krreet
Copy link
Contributor

krreet commented Jan 14, 2022

#11474 (comment)

@shellscape Looks like CRA has its dependencies unpinned very recently. check the related PR.
maybe its not very wise to pin dependencies. There may be patch releases that may fix a vulnerability or critical fixes by the library authors, we don't want to miss.

also
users can pin dependencies themselves if needed via Yarn's resolutions or npm's upcoming overrides feature.

@sauntimo
Copy link

sauntimo commented Jan 14, 2022

@jjavierdguezas I ran into this problem today as well, with lots of the following error

Loading PostCSS "postcss-preset-env" plugin failed: Unexpected token '.'

Although my project didn't have postcss-preset-env as a direct dependency, react-scripts@5.0.0 includes postcss-preset-env@7.2.0.

I included the most recent version as a direct dependency and it seems to be okay for now - perhaps not an ideal solution but at least allows me to get on with other things.

yarn add postcss-preset-env@7.2.3

KindaOK added a commit to michiganhackers/mh-web-new that referenced this issue Jan 18, 2022
Updated react to 17. Removed several unused dependencies. Removed some ESLint rules that conflict with prettier. Also moved some Create React App dependencies up to package.json as per facebook/create-react-app#11906 and facebook/create-react-app#11474. Updated the .eslintrc.json file to reflect updates to ESLint and babel. Also removed package-lock.json to prevent confusion. Pain.
@Luchanso
Copy link

Luchanso commented Feb 3, 2022

Yes, please pin your dependenices 🙏 We have other problem - after each packages patch update, we wait 10 - 24 hours, when our private registry pull new package versions 😞

So, now we have problem with babel packages:

npm timing stage:rollbackFailedOptional Completed in 2ms
npm timing stage:runTopLevelLifecycles Completed in 11407ms
npm verb type range
npm verb stack @babel/helpers: No matching version found for @babel/helpers@^7.17.0.
npm verb stack     at pickManifest (/usr/local/lib/node_modules/npm/node_modules/npm-pick-manifest/index.js:122:13)
npm verb stack     at /usr/local/lib/node_modules/npm/node_modules/pacote/lib/fetchers/registry/manifest.js:43:18
npm verb stack     at tryCatcher (/usr/local/lib/node_modules/npm/node_modules/bluebird/js/release/util.js:16:23)
npm verb stack     at Promise._settlePromiseFromHandler (/usr/local/lib/node_modules/npm/node_modules/bluebird/js/release/promise.js:517:31)
npm verb stack     at Promise._settlePromise (/usr/local/lib/node_modules/npm/node_modules/bluebird/js/release/promise.js:574:18)
npm verb stack     at Promise._settlePromise0 (/usr/local/lib/node_modules/npm/node_modules/bluebird/js/release/promise.js:619:10)
npm verb stack     at Promise._settlePromises (/usr/local/lib/node_modules/npm/node_modules/bluebird/js/release/promise.js:699:18)
npm verb stack     at _drainQueueStep (/usr/local/lib/node_modules/npm/node_modules/bluebird/js/release/async.js:138:12)
npm verb stack     at _drainQueue (/usr/local/lib/node_modules/npm/node_modules/bluebird/js/release/async.js:131:9)
npm verb stack     at Async._drainQueues (/usr/local/lib/node_modules/npm/node_modules/bluebird/js/release/async.js:147:5)
npm verb stack     at Immediate.Async.drainQueues [as _onImmediate] (/usr/local/lib/node_modules/npm/node_modules/bluebird/js/release/async.js:17:14)
npm verb stack     at processImmediate (internal/timers.js:464:21)

Yeah, we know thats problem solve via package/yarn lock files, but sometimes we updating our packages or erase package-lock

@romainmenke
Copy link

Please don't do this.
It's up to end users to manage and optionally lock their dependencies.

Pinned versions solve this issue but create another one.

  • you (maintainer of CRA) are AFK
  • a critical security issue is found in a dependency

-> no one can resolve this issue without you taking action

The only real effect pinning versions will have is that you will have more maintenance chores.

@Luchanso
Copy link

@romainmenke I think one of the major reason to not pinning dependencies is project can be himself using the same dependencies, so when dependencies is pinng it can be broke project

By other side, we can't garantie to reproduction (deterministic) build https://12factor.net/dependencies

Yes, thats tradeoff, and after a while I changed my mind - better will be if we don't pin dependenices

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file
Projects
None yet
Development

No branches or pull requests

9 participants