From c5b51f021cad59e38e8244fcba613fd0547eda7f Mon Sep 17 00:00:00 2001 From: Konstantin Kai Date: Fri, 18 Oct 2024 14:47:47 +0300 Subject: [PATCH 1/4] fix(vite): resolves files with dot suffixes correctly concat `ext` with `filename` for parse in `findFile` function from `nx-tsconfig-paths.plugin` that previously led to an unresolved error for files with dot suffixes e.g.`/dir/file.suffix.ext` Closes #27852 --- .../vite/plugins/nx-tsconfig-paths.plugin.ts | 28 +----- .../utils/nx-tsconfig-paths-find-file.spec.ts | 95 +++++++++++++++++++ .../src/utils/nx-tsconfig-paths-find-file.ts | 29 ++++++ 3 files changed, 128 insertions(+), 24 deletions(-) create mode 100644 packages/vite/src/utils/nx-tsconfig-paths-find-file.spec.ts create mode 100644 packages/vite/src/utils/nx-tsconfig-paths-find-file.ts diff --git a/packages/vite/plugins/nx-tsconfig-paths.plugin.ts b/packages/vite/plugins/nx-tsconfig-paths.plugin.ts index cf8d9b3f0c707..18b2098062934 100644 --- a/packages/vite/plugins/nx-tsconfig-paths.plugin.ts +++ b/packages/vite/plugins/nx-tsconfig-paths.plugin.ts @@ -5,7 +5,7 @@ import { workspaceRoot, } from '@nx/devkit'; import { copyFileSync, existsSync } from 'node:fs'; -import { join, parse, relative, resolve } from 'node:path'; +import { join, relative, resolve } from 'node:path'; import { loadConfig, createMatchPath, @@ -18,6 +18,7 @@ import { } from '@nx/js/src/utils/buildable-libs-utils'; import { Plugin } from 'vite'; import { nxViteBuildCoordinationPlugin } from './nx-vite-build-coordination.plugin'; +import { findFile } from '../src/utils/nx-tsconfig-paths-find-file'; import { isUsingTsSolutionSetup } from '@nx/js/src/utils/typescript/ts-solution-setup'; export interface nxViteTsPathsOptions { @@ -243,33 +244,12 @@ There should at least be a tsconfig.base.json or tsconfig.json in the root of th ); resolvedFile = findFile( - importPath.replace(normalizedImport, joinedPath) + importPath.replace(normalizedImport, joinedPath), + options.extensions ); } } return resolvedFile; } - - function findFile(path: string): string { - for (const ext of options.extensions) { - // Support file with "." in the name. - let resolvedPath = resolve(path + ext); - if (existsSync(resolvedPath)) { - return resolvedPath; - } - - // Support file extensions such as .css and .js in the import path. - const { dir, name } = parse(path); - resolvedPath = resolve(dir, name + ext); - if (existsSync(resolvedPath)) { - return resolvedPath; - } - - const resolvedIndexPath = resolve(path, `index${ext}`); - if (existsSync(resolvedIndexPath)) { - return resolvedIndexPath; - } - } - } } diff --git a/packages/vite/src/utils/nx-tsconfig-paths-find-file.spec.ts b/packages/vite/src/utils/nx-tsconfig-paths-find-file.spec.ts new file mode 100644 index 0000000000000..ec8d573e8f847 --- /dev/null +++ b/packages/vite/src/utils/nx-tsconfig-paths-find-file.spec.ts @@ -0,0 +1,95 @@ +import { findFile as findFileMain } from './nx-tsconfig-paths-find-file'; + +describe('@nx/vite nx-tsconfig-paths-find-file', () => { + const extensions = ['.ts', '.js']; + const fs = new Set(); + const existsSyncImpl = (path: string) => fs.has(path); + const findFile = (path: string): string => + findFileMain(path, extensions, existsSyncImpl); + + beforeAll(() => { + [ + '/dir1/file.ts', + '/dir1/file.suffix.ts', + '/dir2/inner/index.ts', + '/dir2/inner/index.js', + '/dir3/file.js', + '/dir4/file.css', + '/dir5/file.suffix.ts.js', + '/dir6/inner.suffix/index.ts', + ].forEach((item) => fs.add(item)); + }); + + afterAll(() => { + fs.clear(); + }); + + const cases: Array<{ + title: string; + path: string; + expected: string | undefined; + }> = [ + { + title: 'Should return undefined for missing file', + path: '/dir10/file', + expected: undefined, + }, + { + title: 'Should return undefined for missing index file', + path: '/dir10/inner', + expected: undefined, + }, + { + title: 'Should return existing file path with extension', + path: '/dir1/file', + expected: '/dir1/file.ts', + }, + { + title: + 'Should return correct file in case with same filename but one with suffix', + path: '/dir1/file.suffix', + expected: '/dir1/file.suffix.ts', + }, + { + title: 'Should return existing file with dir request', + path: '/dir2/inner', + expected: '/dir2/inner/index.ts', + }, + { + title: 'Should return existing file with index request', + path: '/dir2/inner/index', + expected: '/dir2/inner/index.ts', + }, + { + title: 'Should return existing file with js extension', + path: '/dir3/file', + expected: '/dir3/file.js', + }, + { + title: 'Should return undefined for non presented extension', + path: '/dir4/file', + expected: undefined, + }, + { + title: 'Should return undefined for unknown file', + path: '/dir5/file.suffix', + expected: undefined, + }, + { + title: 'Should return js file with strange suffix filename', + path: '/dir5/file.suffix.ts', + expected: '/dir5/file.suffix.ts.js', + }, + { + title: 'Should return index file for dir with suffixed name', + path: '/dir6/inner.suffix', + expected: '/dir6/inner.suffix/index.ts', + }, + ]; + + cases.forEach(({ title, path, expected }) => { + it(title, () => { + expect(findFile(path)).toEqual(expected); + }); + }); +}); diff --git a/packages/vite/src/utils/nx-tsconfig-paths-find-file.ts b/packages/vite/src/utils/nx-tsconfig-paths-find-file.ts new file mode 100644 index 0000000000000..e6860af450dd1 --- /dev/null +++ b/packages/vite/src/utils/nx-tsconfig-paths-find-file.ts @@ -0,0 +1,29 @@ +import { existsSync } from 'node:fs'; +import { parse, resolve } from 'node:path'; + +export function findFile( + /** + * File path without extension + */ + path: string, + extensions: string[], + existsSyncImpl: typeof existsSync = existsSync +): string { + for (const ext of extensions) { + /** + * Support file extensions such as .css and .js in the import path. + * + * NOTE: We should concatenate the `path` with `ext` because filenames with dot suffixes, e.g., `filename.suffix`, resolve incorrectly. + */ + const { dir, name } = parse(path + ext); + const resolvedPath = resolve(dir, name + ext); + if (existsSyncImpl(resolvedPath)) { + return resolvedPath; + } + + const resolvedIndexPath = resolve(path, `index${ext}`); + if (existsSyncImpl(resolvedIndexPath)) { + return resolvedIndexPath; + } + } +} From 2492d7e63db9fddd42f09cba16db22875434b3b6 Mon Sep 17 00:00:00 2001 From: Konstantin Kai Date: Fri, 18 Oct 2024 19:30:55 +0300 Subject: [PATCH 2/4] fix(vite): use basename & dirname instead of node:path parse use basename & dirname in findFile method from nx-tsconfig-paths.plugin --- e2e/vite/src/vite.test.ts | 2 +- .../utils/nx-tsconfig-paths-find-file.spec.ts | 38 ++++++++++++++++--- .../src/utils/nx-tsconfig-paths-find-file.ts | 17 ++++----- 3 files changed, 41 insertions(+), 16 deletions(-) diff --git a/e2e/vite/src/vite.test.ts b/e2e/vite/src/vite.test.ts index cb8e31f36874e..e940d9117dc3c 100644 --- a/e2e/vite/src/vite.test.ts +++ b/e2e/vite/src/vite.test.ts @@ -158,7 +158,7 @@ describe('@nx/vite/plugin', () => { `generate @nx/react:library libs/${mylib} --bundler=none --unitTestRunner=vitest` ); updateFile(`libs/${mylib}/src/styles.css`, `.foo {}`); - updateFile(`libs/${mylib}/src/foo.mts`, `export const foo = 'foo';`); + updateFile(`libs/${mylib}/src/foo.mjs`, `export const foo = 'foo';`); updateFile( `libs/${mylib}/src/foo.spec.ts`, ` diff --git a/packages/vite/src/utils/nx-tsconfig-paths-find-file.spec.ts b/packages/vite/src/utils/nx-tsconfig-paths-find-file.spec.ts index ec8d573e8f847..4886e8bc53e45 100644 --- a/packages/vite/src/utils/nx-tsconfig-paths-find-file.spec.ts +++ b/packages/vite/src/utils/nx-tsconfig-paths-find-file.spec.ts @@ -1,11 +1,11 @@ import { findFile as findFileMain } from './nx-tsconfig-paths-find-file'; describe('@nx/vite nx-tsconfig-paths-find-file', () => { - const extensions = ['.ts', '.js']; + const extensions = ['.ts', '.js', '.mts']; const fs = new Set(); const existsSyncImpl = (path: string) => fs.has(path); - const findFile = (path: string): string => - findFileMain(path, extensions, existsSyncImpl); + const findFile = (path: string, exts: string[] = extensions): string => + findFileMain(path, exts, existsSyncImpl); beforeAll(() => { [ @@ -17,6 +17,7 @@ describe('@nx/vite nx-tsconfig-paths-find-file', () => { '/dir4/file.css', '/dir5/file.suffix.ts.js', '/dir6/inner.suffix/index.ts', + '/file1.mts', ].forEach((item) => fs.add(item)); }); @@ -28,6 +29,7 @@ describe('@nx/vite nx-tsconfig-paths-find-file', () => { title: string; path: string; expected: string | undefined; + extensions?: string[]; }> = [ { title: 'Should return undefined for missing file', @@ -85,11 +87,37 @@ describe('@nx/vite nx-tsconfig-paths-find-file', () => { path: '/dir6/inner.suffix', expected: '/dir6/inner.suffix/index.ts', }, + { + title: 'Should return file for import with extension', + path: '/dir1/file.ts', + expected: '/dir1/file.ts', + }, + { + title: 'Should return file with .js ext instead of .ts', + path: '/dir2/inner/index.js', + expected: '/dir2/inner/index.js', + }, + { + title: 'Should return css file that imported with query', + path: '/dir4/file.css?inline', + expected: '/dir4/file.css', + extensions: ['.js', '.css'], + }, + { + title: 'Should return file with .mts', + path: '/file1.mts', + expected: '/file1.mts', + }, + { + title: 'Should return file', + path: '/file1', + expected: '/file1.mts', + }, ]; - cases.forEach(({ title, path, expected }) => { + cases.forEach(({ title, path, expected, extensions }) => { it(title, () => { - expect(findFile(path)).toEqual(expected); + expect(findFile(path, extensions)).toEqual(expected); }); }); }); diff --git a/packages/vite/src/utils/nx-tsconfig-paths-find-file.ts b/packages/vite/src/utils/nx-tsconfig-paths-find-file.ts index e6860af450dd1..64c1121f4a46c 100644 --- a/packages/vite/src/utils/nx-tsconfig-paths-find-file.ts +++ b/packages/vite/src/utils/nx-tsconfig-paths-find-file.ts @@ -1,21 +1,18 @@ import { existsSync } from 'node:fs'; -import { parse, resolve } from 'node:path'; +import { resolve, basename, dirname } from 'node:path'; export function findFile( - /** - * File path without extension - */ path: string, extensions: string[], existsSyncImpl: typeof existsSync = existsSync ): string { for (const ext of extensions) { - /** - * Support file extensions such as .css and .js in the import path. - * - * NOTE: We should concatenate the `path` with `ext` because filenames with dot suffixes, e.g., `filename.suffix`, resolve incorrectly. - */ - const { dir, name } = parse(path + ext); + // Support file extensions such as .css and .js in the import path. + const [dir, name] = [ + dirname(path), + basename(path.replace(/\?\S*$/, ''), ext), + ]; + const resolvedPath = resolve(dir, name + ext); if (existsSyncImpl(resolvedPath)) { return resolvedPath; From 607bf625105b254555d7ab18cbeb8beaa83be005 Mon Sep 17 00:00:00 2001 From: Konstantin Kai Date: Sun, 20 Oct 2024 12:52:37 +0300 Subject: [PATCH 3/4] fix(vite): use replace only once per function call use replace only once per function in nx-tsconfig-paths-find-file --- packages/vite/src/utils/nx-tsconfig-paths-find-file.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/vite/src/utils/nx-tsconfig-paths-find-file.ts b/packages/vite/src/utils/nx-tsconfig-paths-find-file.ts index 64c1121f4a46c..0f46228dda892 100644 --- a/packages/vite/src/utils/nx-tsconfig-paths-find-file.ts +++ b/packages/vite/src/utils/nx-tsconfig-paths-find-file.ts @@ -6,12 +6,12 @@ export function findFile( extensions: string[], existsSyncImpl: typeof existsSync = existsSync ): string { + const queryLessPath = path.replace(/\?\S*$/, ''); + for (const ext of extensions) { + const dir = dirname(path); // Support file extensions such as .css and .js in the import path. - const [dir, name] = [ - dirname(path), - basename(path.replace(/\?\S*$/, ''), ext), - ]; + const name = basename(queryLessPath, ext); const resolvedPath = resolve(dir, name + ext); if (existsSyncImpl(resolvedPath)) { From 1cbeed56c2e854ecd62369b5d7e687c686f85cbd Mon Sep 17 00:00:00 2001 From: Colum Ferry Date: Thu, 12 Dec 2024 17:12:06 +0000 Subject: [PATCH 4/4] docs(vite): add further comment to explain change --- packages/vite/src/utils/nx-tsconfig-paths-find-file.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/vite/src/utils/nx-tsconfig-paths-find-file.ts b/packages/vite/src/utils/nx-tsconfig-paths-find-file.ts index 0f46228dda892..06a9fcb53b0bb 100644 --- a/packages/vite/src/utils/nx-tsconfig-paths-find-file.ts +++ b/packages/vite/src/utils/nx-tsconfig-paths-find-file.ts @@ -11,6 +11,7 @@ export function findFile( for (const ext of extensions) { const dir = dirname(path); // Support file extensions such as .css and .js in the import path. + // While still allowing for '.suffix' const name = basename(queryLessPath, ext); const resolvedPath = resolve(dir, name + ext);