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

fix(js): ignore bundler config files correctly in eslint config when using vitest in js library generator #29649

Merged
merged 1 commit into from
Jan 16, 2025

Conversation

leosvelperez
Copy link
Member

@leosvelperez leosvelperez commented Jan 16, 2025

Current Behavior

Generating a js library with esbuild or rollup as bundlers and vitest as the unit test runner, results in an eslint configuration where the bundler config file is not ignored from the @nx/dependency-checks rule.

Expected Behavior

Generating a js library with esbuild or rollup as bundlers and vitest as the unit test runner, should result in an eslint configuration where the bundler config file is ignored from the @nx/dependency-checks rule.

Related Issue(s)

Fixes #

@leosvelperez leosvelperez self-assigned this Jan 16, 2025
@leosvelperez leosvelperez requested a review from a team as a code owner January 16, 2025 14:11
Copy link

vercel bot commented Jan 16, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Skipped Deployment
Name Status Preview Updated (UTC)
nx-dev ⬜️ Ignored (Inspect) Jan 16, 2025 2:11pm

Copy link

nx-cloud bot commented Jan 16, 2025

View your CI Pipeline Execution ↗ for commit 40d9f53.

Command Status Duration Result
nx affected --targets=lint,test,build,e2e,e2e-c... ✅ Succeeded 36m 49s View ↗
nx run-many -t check-imports check-commit check... ✅ Succeeded 55s View ↗
nx-cloud record -- nx-cloud conformance:check ✅ Succeeded 1s View ↗
nx-cloud record -- nx format:check --base=ca1c3... ✅ Succeeded 21s View ↗
nx-cloud record -- nx sync:check ✅ Succeeded 20s View ↗
nx documentation --no-dte ✅ Succeeded 39s View ↗

☁️ Nx Cloud last updated this comment at 2025-01-16 14:51:02 UTC

@leosvelperez leosvelperez enabled auto-merge (squash) January 16, 2025 14:21
'{projectRoot}/vite.config.{js,ts,mjs,mts}'
);
o.rules['@nx/dependency-checks'] = [ruleSeverity, ruleOptions];
const ignoredFiles = new Set(ruleOptions.ignoredFiles ?? []);
Copy link
Member

@jaysoo jaysoo Jan 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Technically you could remove the else if from 477 and achieve the same result.

if (options.bundler === "vite" || options.unitTestRunner === 'vitest') {
  ignoredFiles.add("{projectRoot}/vite.config.{js,ts,mjs,mts}");
}

if (options.bundler === "rollup") {
  ignoredFiles.add("{projectRoot}/rollup.config.{js,ts,mjs,mts,cjs,cts}");
} else if (options.bundler === "esbuild") {
  ignoredFiles.add("{projectRoot}/esbuild.config.{js,ts,mjs,mts}");
}

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It works, but I don't think it reads as well because you already check for bundler === vite first, so, there's no way it can be bundler === rollup if that was the case. Checking for bundlers and unit test runners separately feels better to me.

@leosvelperez leosvelperez merged commit 8a6e44b into master Jan 16, 2025
6 checks passed
@leosvelperez leosvelperez deleted the js/ignore-rollup-config-eslint branch January 16, 2025 14:51
FrozenPandaz pushed a commit that referenced this pull request Jan 17, 2025
…using vitest in js library generator (#29649)

## Current Behavior

Generating a js library with esbuild or rollup as bundlers and vitest as
the unit test runner, results in an eslint configuration where the
bundler config file is not ignored from the `@nx/dependency-checks`
rule.

## Expected Behavior

Generating a js library with esbuild or rollup as bundlers and vitest as
the unit test runner, should result in an eslint configuration where the
bundler config file is ignored from the `@nx/dependency-checks` rule.

## Related Issue(s)

Fixes #

(cherry picked from commit 8a6e44b)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants