-
Notifications
You must be signed in to change notification settings - Fork 6.1k
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
feat: add eslint-plugin-jest to backend template #15050
Conversation
Add eslint-plugin-jest to new backend plugins created by `npx backstage-cli new`. When creating a new backstage plugin it prints an error as such: ``` Error: Failed to load plugin 'jest' declared in '.eslintrc.js': Cannot find module 'eslint-plugin-jest' Require stack: - ...omitted.../plugin-backend/__placeholder__.js Referenced from: ...omitted.../plugin-backend/.eslintrc.js Warning: Failed to execute command yarn lint --fix 🎉 Successfully created backend-plugin ``` This aims to fix that by including eslint-plugin-jest as a devDependency Signed-off-by: Rickard Dybeck <dybeck@spotify.com>
Changed Packages
|
@@ -40,6 +40,7 @@ | |||
"devDependencies": { | |||
"@backstage/cli": "{{versionQuery '@backstage/cli'}}", | |||
"@types/supertest": "{{versionQuery '@types/supertest' '2.0.12'}}", | |||
"eslint-plugin-jest": "{{versionQuery 'eslint-plugin-jest' '27.1.6'}}", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is already a dependency of the CLI, so shouldn't be needed
backstage/packages/cli/package.json
Line 78 in 09950be
"eslint-plugin-jest": "^27.0.0", |
What might be going on here is that the node_modules
layout ends up in in such a shape that the ESLint plugin isn't reachable from the plugin itself. The workaround should be to make sure that the ESLint config bundled with the CLI resolves the plugins relative to the config itself, which means inserting require.resolve
over here:
'plugin:jest/recommended', |
backstage/packages/cli/config/jest.js
Line 57 in 09950be
return { testEnvironment: require.resolve('jest-environment-jsdom') }; |
Do you want to dig into figuring out how to get that set up instead? otherwise I could get an issue going
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can take a look, but my nodejs tooling knowledge is not as good as it is for other languages
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it would seem like adding this would also work - but it's dependent on filesystem path
❯ git --no-pager diff
diff --git plugins/plugin-backen/.eslintrc.js plugins/plugin-backend/.eslintrc.js
index e2a53a6..a9a1fbe 100644
--- plugins/plugin-backend/.eslintrc.js
+++ plugins/plugin-backend/.eslintrc.js
@@ -1 +1,3 @@
-module.exports = require('@backstage/cli/config/eslint-factory')(__dirname);
+module.exports = require('@backstage/cli/config/eslint-factory')(__dirname, {
+ extends: ['../../.eslintrc.js'],
+});
edit: never mind, doesn't seem to work
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had another look at this and it turns out ESLint doesn't support my suggested solution of resolving absolute paths, or more like actively refuses it. Right now the proposed solution is to use peer dependencies for plugins and then tools to automatically install peer dependencies, I really don't want to do that because that'll mess with other deps 😅. A possible solution could be to separate out the ESLint config into its own package as that might flatten the dependency graph in some useful ways, but not sure.
Either way, it turns out that the people over at ESLint have been working on a new an improved config format that as far as I can tell will solve this issue and a lot of the other ones that we're seeing when it comes to configuring ESLint for a monorepo. The RFC is over at #15050, and implementation is underway in eslint/eslint#13481. Any changes we do to the way our ESLint config is packages and set up should be taking this new config format into account.
This PR has been automatically marked as stale because it has not had recent activity from the author. It will be closed if no further activity occurs. If the PR was closed and you want it re-opened, let us know and we'll re-open the PR so that you can continue the contribution! |
Hey, I just made a Pull Request!
Add eslint-plugin-jest to new backend plugins created by
npx backstage-cli new
.When creating a new backstage plugin it prints an error as such:
This aims to fix that by including eslint-plugin-jest as a devDependency
Signed-off-by: Rickard Dybeck dybeck@spotify.com
✔️ Checklist
Signed-off-by
line in the message. (more info)