-
-
Notifications
You must be signed in to change notification settings - Fork 9.8k
Presets: Support extensionless imports in TS-based presets #32641
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 7 commits
9503508
c0224d3
522d5f1
6047b63
1b74b96
6865915
cf66625
21e0bcd
029c938
6f3f65f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,288 @@ | ||
| import { existsSync } from 'node:fs'; | ||
|
|
||
| import { beforeEach, describe, expect, it, vi } from 'vitest'; | ||
|
|
||
| import { deprecate } from 'storybook/internal/node-logger'; | ||
|
|
||
| import { addExtensionsToRelativeImports, resolveWithExtension } from './loader'; | ||
|
|
||
| // Mock dependencies | ||
| vi.mock('node:fs'); | ||
| vi.mock('storybook/internal/node-logger'); | ||
|
|
||
| describe('loader', () => { | ||
| describe('resolveWithExtension', () => { | ||
| it('should return the path as-is if it already has an extension', () => { | ||
| const result = resolveWithExtension('./test.js', '/project/src/file.ts'); | ||
|
|
||
| expect(result).toBe('./test.js'); | ||
| expect(deprecate).not.toHaveBeenCalled(); | ||
| }); | ||
|
|
||
| it('should resolve extensionless import to .ts extension when file exists', () => { | ||
| vi.mocked(existsSync).mockImplementation((filePath) => { | ||
| return filePath === '/project/src/utils.ts'; | ||
| }); | ||
|
|
||
| const result = resolveWithExtension('./utils', '/project/src/file.ts'); | ||
|
|
||
| expect(result).toBe('./utils.ts'); | ||
|
Check failure on line 29 in code/core/src/bin/loader.test.ts
|
||
| expect(deprecate).toHaveBeenCalledWith( | ||
| expect.stringContaining('One or more extensionless imports detected: "./utils"') | ||
| ); | ||
| expect(deprecate).toHaveBeenCalledWith( | ||
| expect.stringContaining( | ||
| 'For maximum compatibility, you should add an explicit file extension' | ||
| ) | ||
| ); | ||
| }); | ||
|
|
||
| it('should resolve extensionless import to .js extension when file exists', () => { | ||
| vi.mocked(existsSync).mockImplementation((filePath) => { | ||
| return filePath === '/project/src/utils.js'; | ||
| }); | ||
|
|
||
| const result = resolveWithExtension('./utils', '/project/src/file.ts'); | ||
|
|
||
| expect(result).toBe('./utils.js'); | ||
|
Check failure on line 47 in code/core/src/bin/loader.test.ts
|
||
| expect(deprecate).toHaveBeenCalledWith( | ||
| expect.stringContaining('One or more extensionless imports detected: "./utils"') | ||
| ); | ||
| }); | ||
|
|
||
| it('should show deprecation message when encountering an extensionless import', () => { | ||
| vi.mocked(existsSync).mockReturnValue(true); | ||
|
|
||
| resolveWithExtension('./utils', '/project/src/file.ts'); | ||
|
|
||
| expect(deprecate).toHaveBeenCalledWith( | ||
| expect.stringContaining('One or more extensionless imports detected: "./utils"') | ||
| ); | ||
| expect(deprecate).toHaveBeenCalledWith( | ||
| expect.stringContaining('in file "/project/src/file.ts"') | ||
| ); | ||
| }); | ||
|
|
||
| it('should return original path when file cannot be resolved', () => { | ||
| vi.mocked(existsSync).mockReturnValue(false); | ||
|
|
||
| const result = resolveWithExtension('./missing', '/project/src/file.ts'); | ||
|
|
||
| expect(result).toBe('./missing'); | ||
| expect(deprecate).toHaveBeenCalledWith( | ||
| expect.stringContaining('One or more extensionless imports detected: "./missing"') | ||
| ); | ||
| }); | ||
|
|
||
| it('should resolve relative to parent directory', () => { | ||
| vi.mocked(existsSync).mockImplementation((filePath) => { | ||
| return filePath === '/project/utils.ts'; | ||
| }); | ||
|
|
||
| const result = resolveWithExtension('../utils', '/project/src/file.ts'); | ||
|
|
||
| expect(result).toBe('../utils.ts'); | ||
|
Check failure on line 84 in code/core/src/bin/loader.test.ts
|
||
| expect(deprecate).toHaveBeenCalledWith( | ||
| expect.stringContaining('One or more extensionless imports detected: "../utils"') | ||
| ); | ||
| }); | ||
| }); | ||
JReinhold marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| describe('addExtensionsToRelativeImports', () => { | ||
| beforeEach(() => { | ||
| // Default: all files exist with .ts extension | ||
| vi.mocked(existsSync).mockImplementation((filePath) => { | ||
| return (filePath as string).endsWith('.ts'); | ||
| }); | ||
| }); | ||
|
|
||
| it('should not modify imports that already have extensions', () => { | ||
| const testCases = [ | ||
| { input: `import foo from './test.js';`, expected: `import foo from './test.js';` }, | ||
| { input: `import foo from './test.ts';`, expected: `import foo from './test.ts';` }, | ||
| { input: `import foo from '../utils.mjs';`, expected: `import foo from '../utils.mjs';` }, | ||
| { | ||
| input: `export { bar } from './module.tsx';`, | ||
| expected: `export { bar } from './module.tsx';`, | ||
| }, | ||
| ]; | ||
|
|
||
| testCases.forEach(({ input, expected }) => { | ||
| const result = addExtensionsToRelativeImports(input, '/project/src/file.ts'); | ||
| expect(result).toBe(expected); | ||
| expect(deprecate).not.toHaveBeenCalled(); | ||
| }); | ||
| }); | ||
|
|
||
| it('should add extension to static import statements', () => { | ||
| const source = `import { foo } from './utils';`; | ||
| const result = addExtensionsToRelativeImports(source, '/project/src/file.ts'); | ||
|
|
||
| expect(result).toBe(`import { foo } from './utils.ts';`); | ||
| }); | ||
|
|
||
| it('should add extension to static export statements', () => { | ||
| const source = `export { foo } from './utils';`; | ||
| const result = addExtensionsToRelativeImports(source, '/project/src/file.ts'); | ||
|
|
||
| expect(result).toBe(`export { foo } from './utils.ts';`); | ||
| }); | ||
|
|
||
| it('should add extension to dynamic import statements', () => { | ||
| const source = `const module = await import('./utils');`; | ||
| const result = addExtensionsToRelativeImports(source, '/project/src/file.ts'); | ||
|
|
||
| expect(result).toBe(`const module = await import('./utils.ts');`); | ||
| }); | ||
|
|
||
| it('should handle default imports', () => { | ||
| const source = `import foo from './module';`; | ||
| const result = addExtensionsToRelativeImports(source, '/project/src/file.ts'); | ||
|
|
||
| expect(result).toBe(`import foo from './module.ts';`); | ||
| }); | ||
|
|
||
| it('should handle named imports', () => { | ||
| const source = `import { foo, bar } from './module';`; | ||
| const result = addExtensionsToRelativeImports(source, '/project/src/file.ts'); | ||
|
|
||
| expect(result).toBe(`import { foo, bar } from './module.ts';`); | ||
| }); | ||
|
|
||
| it('should handle namespace imports', () => { | ||
| const source = `import * as utils from './module';`; | ||
| const result = addExtensionsToRelativeImports(source, '/project/src/file.ts'); | ||
|
|
||
| expect(result).toBe(`import * as utils from './module.ts';`); | ||
| }); | ||
|
|
||
| it('should handle side-effect imports', () => { | ||
| const source = `import './styles';`; | ||
| const result = addExtensionsToRelativeImports(source, '/project/src/file.ts'); | ||
|
|
||
| expect(result).toBe(`import './styles.ts';`); | ||
| }); | ||
|
|
||
| it('should handle export all statements', () => { | ||
| const source = `export * from './module';`; | ||
| const result = addExtensionsToRelativeImports(source, '/project/src/file.ts'); | ||
|
|
||
| expect(result).toBe(`export * from './module.ts';`); | ||
| }); | ||
|
|
||
| it('should not modify absolute imports', () => { | ||
| const testCases = [ | ||
| `import foo from 'react';`, | ||
| `import bar from '@storybook/react';`, | ||
| `import baz from 'node:fs';`, | ||
| ]; | ||
|
|
||
| testCases.forEach((source) => { | ||
| const result = addExtensionsToRelativeImports(source, '/project/src/file.ts'); | ||
| expect(result).toBe(source); | ||
| }); | ||
| }); | ||
|
|
||
| it('should not modify imports that match the pattern but are not relative paths', () => { | ||
| // Edge case: a path that starts with a dot but not ./ or ../ | ||
| // This tests the condition that returns 'match' unchanged | ||
| const source = `import foo from '.config';`; | ||
| const result = addExtensionsToRelativeImports(source, '/project/src/file.ts'); | ||
|
|
||
| // Should not be modified since it doesn't start with ./ or ../ | ||
| expect(result).toBe(source); | ||
| expect(deprecate).not.toHaveBeenCalled(); | ||
| }); | ||
|
|
||
| it('should handle single quotes', () => { | ||
| const source = `import foo from './utils';`; | ||
| const result = addExtensionsToRelativeImports(source, '/project/src/file.ts'); | ||
|
|
||
| expect(result).toBe(`import foo from './utils.ts';`); | ||
| }); | ||
|
|
||
| it('should handle double quotes', () => { | ||
| const source = `import foo from "./utils";`; | ||
| const result = addExtensionsToRelativeImports(source, '/project/src/file.ts'); | ||
|
|
||
| expect(result).toBe(`import foo from "./utils.ts";`); | ||
| }); | ||
|
|
||
| it('should handle paths starting with ./', () => { | ||
| const source = `import foo from './utils';`; | ||
| const result = addExtensionsToRelativeImports(source, '/project/src/file.ts'); | ||
|
|
||
| expect(result).toBe(`import foo from './utils.ts';`); | ||
| }); | ||
|
|
||
| it('should handle paths starting with ../', () => { | ||
| const source = `import foo from '../utils';`; | ||
| const result = addExtensionsToRelativeImports(source, '/project/src/file.ts'); | ||
|
|
||
| expect(result).toBe(`import foo from '../utils.ts';`); | ||
| }); | ||
|
|
||
| it('should handle multiple imports in the same file', () => { | ||
| const source = `import foo from './foo';\nimport bar from './bar';\nexport { baz } from '../baz';`; | ||
| const result = addExtensionsToRelativeImports(source, '/project/src/file.ts'); | ||
|
|
||
| expect(result).toBe( | ||
| `import foo from './foo.ts';\nimport bar from './bar.ts';\nexport { baz } from '../baz.ts';` | ||
| ); | ||
| }); | ||
|
|
||
| it('should preserve the import structure after adding extensions', () => { | ||
| const source = `import { foo, bar } from './utils';`; | ||
| const result = addExtensionsToRelativeImports(source, '/project/src/file.ts'); | ||
|
|
||
| expect(result).toContain('{ foo, bar }'); | ||
| expect(result).toBe(`import { foo, bar } from './utils.ts';`); | ||
| }); | ||
|
|
||
| it('should handle imports with comments', () => { | ||
| const source = `// This is a comment\nimport foo from './utils'; // inline comment`; | ||
| const result = addExtensionsToRelativeImports(source, '/project/src/file.ts'); | ||
|
|
||
| expect(result).toBe(`// This is a comment\nimport foo from './utils.ts'; // inline comment`); | ||
| }); | ||
|
|
||
| it('should handle multi-line imports with named exports on separate lines', () => { | ||
| const source = `import { | ||
| foo, | ||
| bar, | ||
| baz | ||
| } from './utils';`; | ||
| const result = addExtensionsToRelativeImports(source, '/project/src/file.ts'); | ||
|
|
||
| expect(result).toBe(`import { | ||
| foo, | ||
| bar, | ||
| baz | ||
| } from './utils.ts';`); | ||
| }); | ||
|
|
||
| it('should handle multi-line exports with named exports on separate lines', () => { | ||
| const source = `export { | ||
| foo, | ||
| bar | ||
| } from '../module';`; | ||
| const result = addExtensionsToRelativeImports(source, '/project/src/file.ts'); | ||
|
|
||
| expect(result).toBe(`export { | ||
| foo, | ||
| bar | ||
| } from '../module.ts';`); | ||
| }); | ||
|
|
||
| it('should handle multi-line dynamic imports', () => { | ||
| const source = `const module = await import( | ||
| './utils' | ||
| );`; | ||
| const result = addExtensionsToRelativeImports(source, '/project/src/file.ts'); | ||
|
|
||
| expect(result).toBe(`const module = await import( | ||
| './utils.ts' | ||
| );`); | ||
| }); | ||
|
Comment on lines
+287
to
+296
Member
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. Should there be a little bit more coverage on off patterns with import(`./foo`); // backticks
import('./foo' + foo);
import(foo + 'bar');
import(`${foo}/bar`);
import(`./${foo}/bar`);
const [] = [
import('./foo'),
import('./bar'),
]
Contributor
Author
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. I've added support for backticks, but supporting interpolation with runtime values doesn't make much sense, since we'd now have to start looking up those runtime values - probably with AST parsing - do figure out what the final file path is to lookup. that's not worth it imo. |
||
| }); | ||
| }); | ||
Uh oh!
There was an error while loading. Please reload this page.