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

add @backstage/eslint-plugin #16174

Merged
merged 25 commits into from Feb 6, 2023
Merged

add @backstage/eslint-plugin #16174

merged 25 commits into from Feb 6, 2023

Conversation

Rugvip
Copy link
Member

@Rugvip Rugvip commented Feb 4, 2023

Hey, I just made a Pull Request!

Broke this out of #15816 as it was getting a bit out of hand.

This now adds 3 new rules:

Rule Description
@backstage/no-forbidden-package-imports Disallow internal monorepo imports from package subpaths that are not exported.
@backstage/no-relative-monorepo-imports Forbid relative imports that reach outside of the package in a monorepo.
@backstage/no-undeclared-imports Forbid imports of external packages that have not been declared in the appropriate dependencies field in package.json.

All in all it adds support for the "exports" field in package.json, bakes in the relative ../*/src check, and replaces the monorepo plugin with our own that works regardless of cwd, fixing #16074.

Custom rules also let us provide more accurate error messages, for example:

lodash must be declared in dependencies of packages/app/package.json, run 'yarn --cwd packages/app add lodash' from the project root.

The replacement of the import/no-extraneous-dependencies rule also comes with a decent speed boost, as it was by far the slowest rule.

before:

yarn lint:all  101.85s user 9.80s system 639% cpu 17.463 total
yarn lint:all  101.67s user 9.93s system 638% cpu 17.491 total
yarn lint:all  100.20s user 9.98s system 644% cpu 17.089 total

Rule                              | Time (ms) | Relative
:---------------------------------|----------:|--------:
import/no-extraneous-dependencies |  1320.333 |    21.0%
@typescript-eslint/no-redeclare   |   798.291 |    12.7%
monorepo/no-relative-import       |   458.716 |     7.3%
notice/notice                     |   318.667 |     5.1%
react/no-multi-comp               |   184.161 |     2.9%
jest/expect-expect                |   181.801 |     2.9%
monorepo/no-internal-import       |   159.004 |     2.5%
react/sort-comp                   |   126.132 |     2.0%
jest/no-standalone-expect         |   118.181 |     1.9%
jest/no-conditional-expect        |   115.060 |     1.8%

after:

yarn lint:all  98.91s user 7.84s system 622% cpu 17.152 total
yarn lint:all  92.07s user 8.19s system 617% cpu 16.229 total
yarn lint:all  97.77s user 7.90s system 634% cpu 16.664 total

Rule                                    | Time (ms) | Relative
:---------------------------------------|----------:|--------:
@typescript-eslint/no-redeclare         |   981.920 |    17.7%
notice/notice                           |   458.716 |     8.3%
@backstage/no-undeclared-imports        |   273.522 |     4.9%
@backstage/no-relative-monorepo-imports |   264.021 |     4.8%
jest/expect-expect                      |   230.718 |     4.2%
react/no-multi-comp                     |   190.179 |     3.4%
jest/no-standalone-expect               |   134.748 |     2.4%
no-restricted-globals                   |   133.390 |     2.4%
jest/no-conditional-expect              |   115.549 |     2.1%
react/sort-comp                         |   113.047 |     2.0%

✔️ Checklist

  • A changeset describing the change and affected packages. (more info)
  • Added or updated documentation
  • Tests for new functionality and regression tests for bug fixes
  • Screenshots attached (for UI changes)
  • All your commits have a Signed-off-by line in the message. (more info)

Signed-off-by: Patrik Oldsberg <poldsberg@gmail.com>
Signed-off-by: Patrik Oldsberg <poldsberg@gmail.com>
Signed-off-by: Patrik Oldsberg <poldsberg@gmail.com>
Signed-off-by: Patrik Oldsberg <poldsberg@gmail.com>
Signed-off-by: Patrik Oldsberg <poldsberg@gmail.com>
Signed-off-by: Patrik Oldsberg <poldsberg@gmail.com>
Signed-off-by: Patrik Oldsberg <poldsberg@gmail.com>
Signed-off-by: Patrik Oldsberg <poldsberg@gmail.com>
Signed-off-by: Patrik Oldsberg <poldsberg@gmail.com>
Signed-off-by: Patrik Oldsberg <poldsberg@gmail.com>
Signed-off-by: Patrik Oldsberg <poldsberg@gmail.com>
Signed-off-by: Patrik Oldsberg <poldsberg@gmail.com>
Signed-off-by: Patrik Oldsberg <poldsberg@gmail.com>
Signed-off-by: Patrik Oldsberg <poldsberg@gmail.com>
Signed-off-by: Patrik Oldsberg <poldsberg@gmail.com>
Signed-off-by: Patrik Oldsberg <poldsberg@gmail.com>
Signed-off-by: Patrik Oldsberg <poldsberg@gmail.com>
Signed-off-by: Patrik Oldsberg <poldsberg@gmail.com>
Signed-off-by: Patrik Oldsberg <poldsberg@gmail.com>
Signed-off-by: Patrik Oldsberg <poldsberg@gmail.com>
Signed-off-by: Patrik Oldsberg <poldsberg@gmail.com>
@Rugvip Rugvip requested review from a team and backstage-service as code owners February 4, 2023 10:54
@github-actions github-actions bot added area:catalog Related to the Catalog Project Area area:scaffolder Everything and all things related to the scaffolder project area labels Feb 4, 2023
@backstage-goalie
Copy link
Contributor

backstage-goalie bot commented Feb 4, 2023

Missing Changesets

The following package(s) are changed by this PR but do not have a changeset:

  • @backstage/backend-defaults
  • @backstage/core-plugin-api
  • @techdocs/cli
  • @backstage/plugin-catalog-backend-module-incremental-ingestion
  • @backstage/plugin-cost-insights
  • @backstage/plugin-scaffolder-backend
  • @backstage/plugin-splunk-on-call

See CONTRIBUTING.md for more information about how to add changesets.

Changed Packages

Package Name Package Path Changeset Bump Current Version
example-app packages/app none v0.2.80-next.1
@backstage/backend-defaults packages/backend-defaults none v0.1.7-next.1
@backstage/backend-test-utils packages/backend-test-utils patch v0.1.34-next.1
@backstage/cli packages/cli patch v0.22.2-next.0
@backstage/codemods packages/codemods patch v0.1.42
@backstage/core-plugin-api packages/core-plugin-api none v1.3.0
@backstage/create-app packages/create-app patch v0.4.37-next.1
e2e-test packages/e2e-test none v0.2.0
@backstage/eslint-plugin packages/eslint-plugin minor v0.0.0
@backstage/repo-tools packages/repo-tools patch v0.1.1
techdocs-cli-embedded-app packages/techdocs-cli-embedded-app none v0.2.79-next.1
@techdocs/cli packages/techdocs-cli none v1.3.2-next.1
@backstage/plugin-catalog-backend-module-incremental-ingestion plugins/catalog-backend-module-incremental-ingestion none v0.2.0-next.1
@backstage/plugin-cost-insights plugins/cost-insights none v0.12.4-next.1
@backstage/plugin-scaffolder-backend plugins/scaffolder-backend none v1.11.0-next.1
@backstage/plugin-splunk-on-call plugins/splunk-on-call none v0.4.4-next.1

@Rugvip Rugvip linked an issue Feb 4, 2023 that may be closed by this pull request
2 tasks
@github-actions
Copy link
Contributor

github-actions bot commented Feb 4, 2023

Uffizzi Preview deployment-13522 was deleted.

…ublished package

Signed-off-by: Patrik Oldsberg <poldsberg@gmail.com>
Signed-off-by: Patrik Oldsberg <poldsberg@gmail.com>
Signed-off-by: Patrik Oldsberg <poldsberg@gmail.com>
Signed-off-by: Patrik Oldsberg <poldsberg@gmail.com>
@Rugvip Rugvip merged commit 762f4f6 into master Feb 6, 2023
@Rugvip Rugvip deleted the rugvip/rules branch February 6, 2023 13:05
@github-actions
Copy link
Contributor

github-actions bot commented Feb 6, 2023

Thank you for contributing to Backstage! The changes in this pull request will be part of the 1.11.0 release, scheduled for Tue, 14 Feb 2023.

@benjdlambert benjdlambert mentioned this pull request Feb 14, 2023
@torsami77
Copy link

torsami77 commented Apr 27, 2023

Hi @Rugvip, job well done, I'm having issue with Disallow internal monorepo imports from package subpaths that are not exported. since this update despite exporting package from internal monorepo. Please can you provide a code snippet example or reference, I keep getting @internal/plugin-core-react does not export ....

@Rugvip
Copy link
Member Author

Rugvip commented Apr 27, 2023

@torsami77 thanks! Could you show an example where you get that error? The rule is there to forbid importing of subpaths like @internal/my-plugin/components where /components is the subpath that will only be allowed if it's listed in the package.json exports field.

@torsami77
Copy link

torsami77 commented Apr 27, 2023

Thanks for your quick response @Rugvip, I need an example on how to properly config the export in package.json

Screenshot from 2023-04-27 09-43-20

@Rugvip
Copy link
Member Author

Rugvip commented Apr 27, 2023

@torsami77 that's all over here https://backstage.io/docs/local-dev/cli-build-system#subpath-exports

@torsami77
Copy link

okay thanks, let me take a look

@torsami77
Copy link

torsami77 commented May 3, 2023

@Rugvip , thank you for your help, the reference you provided above helped me resolve the lint error, but now I'm trying to run the app and I'm getting similar issues as on #13982

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:catalog Related to the Catalog Project Area area:scaffolder Everything and all things related to the scaffolder project area
Projects
None yet
Development

Successfully merging this pull request may close these issues.

🐛 Bug Report: cli repo lint commands do not catch errors in monorepo rules
5 participants