-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
chore(repo): add eslint and jest plugins to use inferred targets #22946
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 1 Ignored Deployment
|
☁️ Nx Cloud ReportCI is running/has finished running commands for commit 8bff6a5. As they complete they will appear below. Click to see the status, the terminal output, and the build insights. 📂 See all runs for this CI Pipeline Execution ✅ Successfully ran 6 targets
Sent with 💌 from NxCloud. |
e548780
to
11fd6bd
Compare
@@ -28,7 +28,8 @@ | |||
"storybook/no-uninstalled-addons": [ | |||
"error", | |||
{ | |||
"ignore": ["@nx/react/plugins/storybook"] | |||
"ignore": ["@nx/react/plugins/storybook"], | |||
"packageJsonLocation": "../../package.json" |
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.
Need to point the Storybook lint rule to the root package.json
, or else it'll pick up the package one (e.g. packages/vue/package.json
) which results in missing dependencies being reported.
11fd6bd
to
9186a63
Compare
path, | ||
join(options.basedir, 'fake-placeholder.ts'), | ||
compilerOptions, | ||
host | ||
).resolvedModule.resolvedFileName; | ||
if (name.startsWith('..')) { | ||
return path_1.join(options.rootDir, name); |
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.
If the module is a package outside of the current project, then resolve it to an absolute path.
This ensures that jest.mock('@nx/devkit)
, for example, resolves to /Users/foo/projects/nx/packages/devkit/src/index.ts
and not ../../../devkit/src/index.ts
. The relative path is not compatiblewith jest.mock
and will lead to modules being unmocked.
9186a63
to
5bff04d
Compare
packages/remix/src/generators/application/__snapshots__/application.impl.spec.ts.snap
Show resolved
Hide resolved
5bff04d
to
608b914
Compare
2ddb660
to
f3fa290
Compare
@@ -90,7 +89,9 @@ export default { | |||
'no-template-curly-in-string': 'warn', | |||
'no-this-before-super': 'warn', | |||
'no-throw-literal': 'warn', | |||
'no-restricted-globals': ['error'].concat(restrictedGlobals), | |||
'no-restricted-globals': ['error'].concat( | |||
require('confusing-browser-globals') |
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.
Because we point to source, the transpile here can lead to errors if we use the default import (due to ESM interopt). Safer to just use require
since this package is CJS-only.
This is an Nx repo issue only.
…o it works in Nx repo with crystal plugins
31f5331
to
6095d06
Compare
@@ -31,8 +33,11 @@ describe('@nx/jest/plugin', () => { | |||
}); | |||
}); | |||
|
|||
test('foo', () => {}); |
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.
Please remove this.
This pull request has already been merged/closed. If you experience issues related to these changes, please open a new issue referencing this pull request. |
This PR adds
@nx/eslint/plugin
and@nx/jest/plugin
to use inferred targets for lint and unit tests. The main changes are removing targets fromproject.json
andnx.json
. There is a fix in thepatch-jest-resolver.js
file to ensure that all modules are resolved to an absolute path, which is needed forjest.mock
to work. A couple of snapshots are updated too withcwd
changes.Note: We can now run any Jest spec file from IDE since the executor and cwd are no longer assumed.