-
Notifications
You must be signed in to change notification settings - Fork 98
Got npm install
Warnings When Use eslint-config-airbnb-base
#6
Comments
Hey @iFwu - thanks for reporting this. I need to convert this into a monorepo, and publish two npm packages to fix this issue. One package with React plugins, and the other "base" without React plugins ( Sorry about that. In the meantime, you are safe to ignore the warnings, everything will still operate as expected. |
@iamturns Until then, |
@iamturns What about defining |
optionalDeps will also be installed. |
@iamturns Sorry for the ping, but it has been one year, are there any update on this? |
@mgenware Unfortunately, the upgrade is not that easy to be pulled off correctly. The Airbnb config is still being updated to ESLint v7 as I'm writing this, and your project also depends on it. This means that unexpected errors may pop up during usage. |
Maybe the best option will be to remove airbnb packages from dependencies. Just in your package describe that it will work only if using airbnb configs. And the You should require from user to install airbnb and set this config in extends before yours package? |
Fixes iamturns#6 by adding downstream peer dependencies to this package (we add the React specific dependencies as optional). This also enables Yarn v2 support (which requires upstream packages to include their dependency's peer dependencies).
Temporary fix if you're using Yarn v2 is just to use the packageExtensions:
[email protected]:
peerDependencies:
eslint: "^5.16.0 || ^6.8.0 || ^7.2.0"
eslint-plugin-import: "^2.21.2"
eslint-plugin-jsx-a11y: "^6.3.0"
eslint-plugin-react: "^7.20.0"
eslint-plugin-react-hooks: "^4 || ^3 || ^2.3.0 || ^1.7.0" |
BREAKING CHANGE: eslint-config-aribnb will need to be installed explicitly, similar to a plugin.
A relatively inexpensive change is removing the dependency for I can put it in as a PR if that's a path you'd like to go down. |
Or split this package as |
Yeah, that's definitely the best long-term solution. I believe that's the concept that @iamturns has in mind from this comment, but it's been a while, so maybe that's a big task. Wanted to offer a low-effort solution (albeit one with a breaking change) |
@iamturns so are you OK to make |
After thinking about this a bit, I think there is no reason to add all that failing peer deps to peer deps meta because this package doesn't depend on it implicitly, So, the proposed solution is to remove it as a dependency completely. Do the same for Specify .eslintrc for base package{
"extends": ["eslint-config-airbnb-base", "eslint-config-airbnb-typescript"]
} .eslintrc for react{
"extends": ["eslint-config-airbnb", "eslint-config-airbnb-typescript"]
} And no more warnings, no need for a separate package and everybody is happy :) Update: anyway, to fix warning |
Workaround for // pnpmfile.js
function readPackage(pkg) {
if (pkg.name === 'eslint-config-airbnb-typescript') {
const { ['eslint-config-airbnb']: airbnb, ...dependencies } = pkg.dependencies;
pkg.dependencies = dependencies;
}
return pkg;
}
module.exports = {
hooks: {
readPackage
}
}; |
if anyone wants to have typescript support for |
@nilzona what are the differences between your project and @mgenware's one here: #6 (comment) ?? |
in the airbnb-base case it's not a huge difference, except that when you install the library from my project you will get everything you need. (airbnb-base + peer dependencies, @typescript/eslint-plugin + parser). I would argue that the setup will be somewhat simpler. The project is also converted into a monorepo, making the setup simpler for the airbnb case also. |
@iamturns would a PR that turns this into a monorepo be helpful or are you feeling like that's not a direction you want to move / you'd want to own that yourself. |
🎉 This issue has been resolved in version 13.0.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
I got warnings when use eslint-config-airbnb-base (no React support).
The text was updated successfully, but these errors were encountered: