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.

Sign up for GitHub

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

Jump to bottom

feat: add eslint-plugin-jest to backend template #15050

Closed
wants to merge 1 commit into from
Closed

feat: add eslint-plugin-jest to backend template #15050

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/yellow-brooms-repeat.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@backstage/cli': minor
---

Include eslint-plugin-jest in scaffolded backstage plugin
1 change: 1 addition & 0 deletions packages/cli/templates/default-backend-plugin/package.json.hbs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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'}}",
Copy link
Member

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:

backstage/packages/cli/config/eslint-factory.js

Line 62 in 09950be

'plugin:jest/recommended',
. I'm not entirely sure what we'd be resolving though tbh, it would need to mimic the way that ESLint looks up these presets. Either way, it'd look something similar to what we do for example in the Jest config:

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

Copy link
Contributor Author

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

Copy link
Contributor Author

@alde alde Dec 6, 2022 •
edited

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

Copy link
Member

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.

"supertest": "{{versionQuery 'supertest' '6.2.4'}}",
"msw": "{{versionQuery 'msw' '0.49.0'}}"
},
Expand Down