fix: move dev dependencies, and specify peer dependencies #443
Conversation
package.json Outdated
| "framer-motion": "2.6.13", | ||
| "i18next": "19.7.0", | ||
| "mime-types": "2.1.26", | ||
| "next": "^9.5.3", |
There was a problem hiding this comment.
Ok really it's my fault. next should also be a devDependency the same for all babel stuff.
There was a problem hiding this comment.
I was also thinking that they should be devDependencies, but then I thought that would break consumers trying to use one of the plugins, e.g. cypress-react-unit-test/plugins/next. I guess it will only break them if they haven't installed next by any other means though. So this might not be a problem?
There was a problem hiding this comment.
Yes it's not a problem, next should be an optional peerDependecy and installed only for us as devDependency
There was a problem hiding this comment.
Moving next to dependencies + peerDependencies seems to be ok, as we can assume that consumers of plugins/next already have next in their own dependencies.
Moving the following dependencies used by plugins/* may not be ok, as not everyone is installing those themselves:
"@babel/plugin-transform-modules-commonjs": "7.10.4", "@cypress/webpack-preprocessor": "5.4.5", "babel-plugin-istanbul": "6.0.0", "find-webpack": "2.1.0", "mime-types": "2.1.26",` Since @cypress/webpack-preprocessor lists the following as peerDependencies, I have to add them to either our dependencies or peerDependencies in order for Yarn 2 to be happy:
"@babel/core": "^7.0.1", "@babel/preset-env": "^7.0.0", "babel-loader": "^8.0.2", "webpack": "^4.18.1" 
There was a problem hiding this comment.
so the top list has to be in the prod dependencies - since this is what we use to find webpack and instrument code, etc. But the bottom list with babel/core, babel/preset-env, babel-loader, and webpack I think we expect to find these dependencies. What's your situation for using cypress-react-unit-test ? Does Yarn complain that webpack is not listed for example?
There was a problem hiding this comment.
@bahmutov Not including the bottom list in dependencies gives the following warnings using Yarn 2:
➤ YN0002: │ cypress-react-unit-test@npm:4.14.1 doesn't provide @babel/core@^7.0.0-0 requested by @babel/plugin-transform-modules-commonjs@npm:7.10.4 ➤ YN0002: │ cypress-react-unit-test@npm:4.14.1 doesn't provide @babel/core@^7.0.1 requested by @cypress/webpack-preprocessor@npm:5.4.5 ➤ YN0002: │ cypress-react-unit-test@npm:4.14.1 doesn't provide @babel/preset-env@^7.0.0 requested by @cypress/webpack-preprocessor@npm:5.4.5 ➤ YN0002: │ cypress-react-unit-test@npm:4.14.1 doesn't provide babel-loader@^8.0.2 requested by @cypress/webpack-preprocessor@npm:5.4.5 ➤ YN0002: │ cypress-react-unit-test@npm:4.14.1 doesn't provide webpack@^4.18.1 requested by @cypress/webpack-preprocessor@npm:5.4.5 If I'm not mistaken, yarn 2 in PnP mode with the default settings will deny require()-calls from cypress-react-unit-test to the above mentioned packages, unless the warnings are fixed.
We can resolve this by putting them in both devDependencies (using the versions you need) and in peerDependencies while also making them optional by specifying them in peerDependenciesMeta.
I'll push a new version with the suggested changes.
package.json Outdated
| "url": "https://github.com/bahmutov/cypress-react-unit-test.git" | ||
| }, | ||
| "dependencies": { | ||
| "@babel/core": "7.4.5", |
There was a problem hiding this comment.
why do we include babel and babel-loader as prod dependencies? I assume the React project has them.
| "@babel/plugin-transform-modules-commonjs": "7.10.4", | ||
| "@cypress/code-coverage": "3.8.1", | ||
| "@cypress/webpack-preprocessor": "5.4.5", | ||
| "@mdx-js/loader": "^1.6.16", |
There was a problem hiding this comment.
agree with next and framer-motion and i18next - these are all for our examples, thus should be dev dependencies
package.json Outdated
| "react-i18next": "11.7.2", | ||
| "unfetch": "4.1.0" | ||
| "unfetch": "4.1.0", | ||
| "webpack": "4.44.1" |
There was a problem hiding this comment.
disagree about webpack - it should be found in the user's project
package.json Outdated
| }, | ||
| "peerDependencies": { | ||
| "cypress": "*", | ||
| "next": "*", |
There was a problem hiding this comment.
so this will complain about next not found for every project?
There was a problem hiding this comment.
I would put webpack here instead
| @larrifax please allow changes from maintainers. I cannot push to your branch using Suggested versions range "peerDependencies": { "cypress": "*", "@babel/core": "^=7.x", "@babel/preset-env": "^=7.x", "babel-loader": "^=8.x", "next": "^=8.x", "react": "^=16.x", "react-dom": "^=16.x", "webpack": "^=3.x" }, |
These packages are peerDependencies of some of the listed dependencies, which means that we have to provide them in our dependencies as well.
…ional peerDependencies This gets rid of the warnings when installing the library into a project using Yarn 2
The |
bahmutov changed the title 
| 🎉 This PR is included in version 4.14.3 🎉 The release is available on: Your semantic-release bot 📦🚀 |
3 participants
devDependencies.I therefore added their
peerDependenciestodependencies, and supplied of them inpeerDependenciesso that package managers will prefer the versions that consumers has installed.Fixes #442