-
-
Notifications
You must be signed in to change notification settings - Fork 10.1k
fix(checklist): respect disableTelemetry config in initializeChecklist #34435
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
Changes from all commits
95e7638
912784a
9dd8d5f
94ed900
c003cb1
f001002
0977032
03d0299
a570e59
acf2b44
285866d
d9288e2
e8ebfd0
8fe8050
02dd879
4ae4460
389cecf
aeba4bf
8c841e6
bde74ae
e77def3
e367333
4ef05ef
841eb65
98cc4b5
89d2e0f
d959d6c
70ac263
8ead8f8
fe31daf
c400893
92ce460
ee7ef00
1f194b1
8d21731
c947cc8
9d9bceb
c25df87
78489c2
8e2b782
2dadfd5
14a9134
e979a25
9c9d3eb
05d356b
65beae2
1053c2a
5d7d985
439af85
2f8d01d
cae3d07
babcf85
69d8d3b
9b895ec
40ef0ea
d09e7f8
cc0d1f9
e7d1d84
3802165
17fcaa9
711e245
a075e07
2f7fcfb
8d687ec
1c96212
0ca7278
78f274b
d9652ff
7c2a587
a36426a
719b6ca
e3021c7
46e5dc7
eb607d3
ca176a0
b119439
f3ccdd9
ec2587d
d58e069
4cdde82
c4c587e
e22f6a0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,49 @@ | ||||||
| import { describe, expect, it } from 'vitest'; | ||||||
|
|
||||||
| import { isConfigAlreadySetup } from './postinstall'; | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion | 🟠 Major Missing file extension on relative import. As per coding guidelines, TypeScript files should use explicit file extensions for relative imports. 🔧 Suggested fix-import { isConfigAlreadySetup } from './postinstall';
+import { isConfigAlreadySetup } from './postinstall.ts';📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||
|
|
||||||
| describe('postinstall helpers', () => { | ||||||
| it('detects a fully configured Vitest config with addon plugin', () => { | ||||||
| const config = ` | ||||||
| import { defineConfig } from 'vitest/config'; | ||||||
| import { storybookTest } from '@storybook/addon-vitest/vitest-plugin'; | ||||||
|
|
||||||
| export default defineConfig({ | ||||||
| test: { | ||||||
| projects: [ | ||||||
| { | ||||||
| extends: true, | ||||||
| plugins: [storybookTest({ configDir: '.storybook' })], | ||||||
| test: { | ||||||
| setupFiles: ['./.storybook/vitest.setup.ts'], | ||||||
| }, | ||||||
| }, | ||||||
| ], | ||||||
| }, | ||||||
| }); | ||||||
| `; | ||||||
|
|
||||||
| expect(isConfigAlreadySetup('/project/vitest.config.ts', config)).toBe(true); | ||||||
| }); | ||||||
|
|
||||||
| it('returns false when storybookTest plugin is not used', () => { | ||||||
| const config = ` | ||||||
| import { defineConfig } from 'vitest/config'; | ||||||
|
|
||||||
| export default defineConfig({ | ||||||
| test: { | ||||||
| projects: [ | ||||||
| { | ||||||
| extends: true, | ||||||
| test: { | ||||||
| setupFiles: ['./.storybook/vitest.setup.ts'], | ||||||
| }, | ||||||
| }, | ||||||
| ], | ||||||
| }, | ||||||
| }); | ||||||
| `; | ||||||
|
|
||||||
| expect(isConfigAlreadySetup('/project/vitest.config.ts', config)).toBe(false); | ||||||
| }); | ||||||
| }); | ||||||
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.
🧩 Analysis chain
🌐 Web query:
In eslint-plugin-import-x (or eslint-plugin-import equivalent), does thenamedrule catch invalid named imports that TypeScript may miss (especially when importing from untyped JS/CJS modules)? Please provide current docs/recommendations.💡 Result:
Yes, the import-x/named rule in eslint-plugin-import-x (and the equivalent import/named in eslint-plugin-import) catches invalid named imports from untyped JS/CJS modules that TypeScript may miss. The rule statically analyzes the targeted module file to verify that named imports match actual named exports declared there, regardless of TypeScript typing. Key points from official docs: - It verifies "named imports are part of the set of named exports in the referenced module" by inspecting the file contents {1, high}. - For untyped JS/CJS modules (especially node_modules), it checks package.json for jsnext:main (deprecated) or module fields to find the ES module entry, then analyzes exports {1, high}. - Modules can be ignored via import-x/ignore setting (defaults include node_modules), but when not ignored, it analyzes them {1, high}. - Note: "A module path that is ignored or not unambiguously an ES module will not be reported" {1, high}. CJS modules with module.exports may not be treated as ES modules, potentially skipping analysis, but pure JS ES modules or those with module field are checked. - In TypeScript configs (flat/typescript, typescript), the rule is disabled (🚫) because TypeScript handles named import validation via types, but it can be manually enabled and works with eslint-import-resolver-typescript for path resolution {1,3,4,5, high}. TypeScript itself may miss runtime exports in untyped JS if no .d.ts declares them exhaustively. Current recommendations (2026): - Use eslint-plugin-import-x with flat/recommended or recommended configs where named is enabled by default {3,4,5, high}. - For TS projects, extend plugin:import-x/typescript + resolver, but enable import-x/named explicitly if you want checks on untyped JS {4,16, high}. - Pair with eslint-import-resolver-typescript to properly resolve paths and prefer
@typesbut fall back to JS analysis {16, high}.Citations:
🏁 Script executed:
Repository: storybookjs/storybook
Length of output: 1433
🏁 Script executed:
rg -A 5 -B 5 "import-x/named" code/.eslintrc.jsRepository: storybookjs/storybook
Length of output: 413
🏁 Script executed:
Repository: storybookjs/storybook
Length of output: 156
🏁 Script executed:
Repository: storybookjs/storybook
Length of output: 379
🏁 Script executed:
sed -n '16,68p' code/.eslintrc.jsRepository: storybookjs/storybook
Length of output: 2247
Clarify the misleading comment about JS file configuration
Lines 163–164: The comment states "It IS set to 'error' for JS files," but this ESLint config has no explicit
import-x/namedrule for JS files. The rule is only mentioned here, where it's disabled for TypeScript files. Revise the comment to accurately reflect what the config does—disable the check for TypeScript (where TypeScript validates named imports) while relying on plugin defaults for JS files.🤖 Prompt for AI Agents