From 5a31cd403d0e76f74d51be4b05af37553e3e6764 Mon Sep 17 00:00:00 2001 From: Greg Signal Date: Sat, 13 Jul 2024 23:57:23 +0200 Subject: [PATCH 1/5] fix: don't resolve tsconfig for externals Original issue: https://github.com/privatenumber/esbuild-loader/issues/363, esbuild-loader would attempt to resolve typescript configs for external sources under some conditions, and cause problems (in this case, the resolved tsconfig.json referenced a config that is not in the dependency tree and thus unresolvable) tsconfig fix: warn on tsconfig resolution error, continue transform --- src/loader.ts | 17 +++++++-- tests/specs/tsconfig.ts | 76 ++++++++++++++++++++++++++++++++++++++++- 2 files changed, 90 insertions(+), 3 deletions(-) diff --git a/src/loader.ts b/src/loader.ts index c8453b3..fe63d0b 100644 --- a/src/loader.ts +++ b/src/loader.ts @@ -81,8 +81,21 @@ async function ESBuildLoader( } else { /* Detect tsconfig file */ - // Webpack shouldn't be loading the same path multiple times so doesn't need to be cached - const tsconfig = getTsconfig(resourcePath, 'tsconfig.json', tsconfigCache); + let tsconfig; + + try { + // Webpack shouldn't be loading the same path multiple times so doesn't need to be cached + tsconfig = getTsconfig(resourcePath, 'tsconfig.json', tsconfigCache); + } catch (error) { + if (resourcePath.split(path.sep).includes('node_modules')) { + this.emitWarning( + new Error(`[esbuild-loader] Error while discovering tsconfig.json in dependency: "${error?.toString()}"`), + ); + } else { + throw error; + } + } + if (tsconfig) { const fileMatcher = createFilesMatcher(tsconfig); transformOptions.tsconfigRaw = fileMatcher(resourcePath) as TransformOptions['tsconfigRaw']; diff --git a/tests/specs/tsconfig.ts b/tests/specs/tsconfig.ts index 57e6a28..38ef915 100644 --- a/tests/specs/tsconfig.ts +++ b/tests/specs/tsconfig.ts @@ -2,7 +2,7 @@ import path from 'path'; import { createRequire } from 'node:module'; import { testSuite, expect } from 'manten'; import { createFixture } from 'fs-fixture'; -import { execa } from 'execa'; +import { execa, type ExecaError } from 'execa'; import { tsconfigJson } from '../utils.js'; const require = createRequire(import.meta.url); @@ -262,6 +262,80 @@ export default testSuite(({ describe }) => { const code2 = await fixture.readFile('dist/index2.js', 'utf8'); expect(code2).toMatch('__publicField(this, "foo", 100);'); }); + + test('ignores tsconfig.json in third party dependencies', async () => { + await using fixture = await createFixture({ + // Fake external dependency + node_modules: { + 'fake-lib': { + 'index.ts': 'export function testFn(): string { return "Hi!" }', + 'package.json': JSON.stringify({ + name: 'fake-lib', + }), + 'tsconfig.json': tsconfigJson({ + extends: 'something-imaginary', + }), + }, + }, + // Our project + src: { + 'index.ts': ` + import { testFn } from "fake-lib"; + + export default testFn;`, + }, + 'webpack.config.js': ` + module.exports = { + mode: 'production', + + optimization: { + minimize: false, + }, + + resolveLoader: { + alias: { + 'esbuild-loader': ${JSON.stringify(esbuildLoader)}, + }, + }, + + resolve: { + extensions: ['.ts', '.js'], + }, + + module: { + rules: [ + { + test: /.[tj]sx?$/, + loader: 'esbuild-loader', + options: { + target: 'es2015', + } + } + ], + }, + + entry: { + index: './src/index.ts', + }, + }; + `, + }); + + let result; + + try { + result = await execa(webpackCli, { + cwd: fixture.path, + }); + } catch (error) { + result = error as ExecaError; + } + const { exitCode, stdout } = result; + + // We log this as a warning, and continue the transform + expect(stdout).toMatch("Error while discovering tsconfig.json in dependency: \"Error: File 'something-imaginary' not found.\""); + expect(exitCode).toEqual(0); + }); }); describe('plugin', ({ test }) => { From cb3ad94fcae1afafaffbe6847575914db6d2ce7c Mon Sep 17 00:00:00 2001 From: Hiroki Osame Date: Tue, 16 Jul 2024 19:32:11 +0900 Subject: [PATCH 2/5] wip --- src/loader.ts | 13 ++--- tests/specs/tsconfig.ts | 103 +++++++++++++++++++++++++++++----------- 2 files changed, 81 insertions(+), 35 deletions(-) diff --git a/src/loader.ts b/src/loader.ts index fe63d0b..ac1068a 100644 --- a/src/loader.ts +++ b/src/loader.ts @@ -87,12 +87,13 @@ async function ESBuildLoader( // Webpack shouldn't be loading the same path multiple times so doesn't need to be cached tsconfig = getTsconfig(resourcePath, 'tsconfig.json', tsconfigCache); } catch (error) { - if (resourcePath.split(path.sep).includes('node_modules')) { - this.emitWarning( - new Error(`[esbuild-loader] Error while discovering tsconfig.json in dependency: "${error?.toString()}"`), - ); - } else { - throw error; + if (error instanceof Error) { + const tsconfigError = new Error(`[esbuild-loader] Error parsing tsconfig.json:\n${error.message}`); + if (resourcePath.includes(`${path.sep}node_modules${path.sep}`)) { + this.emitWarning(tsconfigError); + } else { + this.emitError(tsconfigError); + } } } diff --git a/tests/specs/tsconfig.ts b/tests/specs/tsconfig.ts index 38ef915..01ee82b 100644 --- a/tests/specs/tsconfig.ts +++ b/tests/specs/tsconfig.ts @@ -2,7 +2,7 @@ import path from 'path'; import { createRequire } from 'node:module'; import { testSuite, expect } from 'manten'; import { createFixture } from 'fs-fixture'; -import { execa, type ExecaError } from 'execa'; +import { execa } from 'execa'; import { tsconfigJson } from '../utils.js'; const require = createRequire(import.meta.url); @@ -263,27 +263,77 @@ export default testSuite(({ describe }) => { expect(code2).toMatch('__publicField(this, "foo", 100);'); }); - test('ignores tsconfig.json in third party dependencies', async () => { + test('fails on invalid tsconfig.json', async () => { await using fixture = await createFixture({ - // Fake external dependency - node_modules: { - 'fake-lib': { - 'index.ts': 'export function testFn(): string { return "Hi!" }', - 'package.json': JSON.stringify({ - name: 'fake-lib', - }), - 'tsconfig.json': tsconfigJson({ - extends: 'something-imaginary', - }), - }, - }, - // Our project + 'tsconfig.json': tsconfigJson({ + extends: 'something-imaginary', + }), src: { 'index.ts': ` - import { testFn } from "fake-lib"; + console.log('Hello, world!' as numer); + `, + }, + 'webpack.config.js': ` + module.exports = { + mode: 'production', - export default testFn;`, + optimization: { + minimize: false, + }, + + resolveLoader: { + alias: { + 'esbuild-loader': ${JSON.stringify(esbuildLoader)}, + }, + }, + + resolve: { + extensions: ['.ts', '.js'], + }, + + module: { + rules: [ + { + test: /.[tj]sx?$/, + loader: 'esbuild-loader', + options: { + target: 'es2015', + } + } + ], + }, + + entry: { + index: './src/index.ts', + }, + }; + `, + }); + + const { stdout, exitCode } = await execa(webpackCli, { + cwd: fixture.path, + reject: false, + }); + + expect(stdout).toMatch('Error parsing tsconfig.json:\nFile \'something-imaginary\' not found.'); + expect(exitCode).toBe(1); + }); + + test('ignores invalid tsconfig.json in dependencies', async () => { + await using fixture = await createFixture({ + 'node_modules/fake-lib': { + 'package.json': JSON.stringify({ + name: 'fake-lib', + }), + 'tsconfig.json': tsconfigJson({ + extends: 'something-imaginary', + }), + 'index.ts': 'export function testFn(): string { return "Hi!" }', }, + 'src/index.ts': ` + import { testFn } from "fake-lib"; + testFn(); + `, 'webpack.config.js': ` module.exports = { mode: 'production', @@ -321,20 +371,15 @@ export default testSuite(({ describe }) => { `, }); - let result; + const { stdout, exitCode } = await execa(webpackCli, { + cwd: fixture.path, + reject: false, + }); - try { - result = await execa(webpackCli, { - cwd: fixture.path, - }); - } catch (error) { - result = error as ExecaError; - } - const { exitCode, stdout } = result; + expect(stdout).toMatch('Error parsing tsconfig.json:\nFile \'something-imaginary\' not found.'); - // We log this as a warning, and continue the transform - expect(stdout).toMatch("Error while discovering tsconfig.json in dependency: \"Error: File 'something-imaginary' not found.\""); - expect(exitCode).toEqual(0); + // Warning so doesn't fail + expect(exitCode).toBe(0); }); }); From 0083a9bf7ffa6cdd285ba98e3efb771b63205e45 Mon Sep 17 00:00:00 2001 From: Hiroki Osame Date: Tue, 16 Jul 2024 19:34:38 +0900 Subject: [PATCH 3/5] wip --- src/loader.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/loader.ts b/src/loader.ts index ac1068a..705a7df 100644 --- a/src/loader.ts +++ b/src/loader.ts @@ -92,7 +92,7 @@ async function ESBuildLoader( if (resourcePath.includes(`${path.sep}node_modules${path.sep}`)) { this.emitWarning(tsconfigError); } else { - this.emitError(tsconfigError); + return done(tsconfigError); } } } From f8c6593a3e95a64c5235824aea3834d30ff90002 Mon Sep 17 00:00:00 2001 From: Hiroki Osame Date: Tue, 16 Jul 2024 19:56:09 +0900 Subject: [PATCH 4/5] wip --- src/loader.ts | 15 ++++++--- tests/specs/tsconfig.ts | 71 +++++++++++++++++++++++++++++++++++++---- 2 files changed, 75 insertions(+), 11 deletions(-) diff --git a/src/loader.ts b/src/loader.ts index 705a7df..181ec3e 100644 --- a/src/loader.ts +++ b/src/loader.ts @@ -35,20 +35,25 @@ async function ESBuildLoader( ); return; } - const transform = implementation?.transform ?? defaultEsbuildTransform; + const { resourcePath } = this; const transformOptions = { ...esbuildTransformOptions, target: options.target ?? 'es2015', loader: options.loader ?? 'default', sourcemap: this.sourceMap, - sourcefile: this.resourcePath, + sourcefile: resourcePath, }; - if (!('tsconfigRaw' in transformOptions)) { - const { resourcePath } = this; + const isDependency = resourcePath.includes(`${path.sep}node_modules${path.sep}`); + if ( + !('tsconfigRaw' in transformOptions) + // If file is local project, always try to apply tsconfig.json (e.g. allowJs) + // If file is dependency, only apply tsconfig.json if .ts + && (!isDependency || resourcePath.endsWith('.ts')) + ) { /** * If a tsconfig.json path is specified, force apply it * Same way a provided tsconfigRaw is applied regardless @@ -89,7 +94,7 @@ async function ESBuildLoader( } catch (error) { if (error instanceof Error) { const tsconfigError = new Error(`[esbuild-loader] Error parsing tsconfig.json:\n${error.message}`); - if (resourcePath.includes(`${path.sep}node_modules${path.sep}`)) { + if (isDependency) { this.emitWarning(tsconfigError); } else { return done(tsconfigError); diff --git a/tests/specs/tsconfig.ts b/tests/specs/tsconfig.ts index 01ee82b..f0b0b63 100644 --- a/tests/specs/tsconfig.ts +++ b/tests/specs/tsconfig.ts @@ -266,7 +266,7 @@ export default testSuite(({ describe }) => { test('fails on invalid tsconfig.json', async () => { await using fixture = await createFixture({ 'tsconfig.json': tsconfigJson({ - extends: 'something-imaginary', + extends: 'unresolvable-dep', }), src: { 'index.ts': ` @@ -315,18 +315,78 @@ export default testSuite(({ describe }) => { reject: false, }); - expect(stdout).toMatch('Error parsing tsconfig.json:\nFile \'something-imaginary\' not found.'); + expect(stdout).toMatch('Error parsing tsconfig.json:\nFile \'unresolvable-dep\' not found.'); expect(exitCode).toBe(1); }); - test('ignores invalid tsconfig.json in dependencies', async () => { + test('ignores invalid tsconfig.json in JS dependencies', async () => { await using fixture = await createFixture({ 'node_modules/fake-lib': { 'package.json': JSON.stringify({ name: 'fake-lib', }), 'tsconfig.json': tsconfigJson({ - extends: 'something-imaginary', + extends: 'unresolvable-dep', + }), + 'index.js': 'export function testFn() { return "Hi!" }', + }, + 'src/index.ts': ` + import { testFn } from "fake-lib"; + testFn(); + `, + 'webpack.config.js': ` + module.exports = { + mode: 'production', + + optimization: { + minimize: false, + }, + + resolveLoader: { + alias: { + 'esbuild-loader': ${JSON.stringify(esbuildLoader)}, + }, + }, + + resolve: { + extensions: ['.ts', '.js'], + }, + + module: { + rules: [ + { + test: /.[tj]sx?$/, + loader: 'esbuild-loader', + options: { + target: 'es2015', + } + } + ], + }, + + entry: { + index: './src/index.ts', + }, + }; + `, + }); + + const { stdout, exitCode } = await execa(webpackCli, { + cwd: fixture.path, + }); + + expect(stdout).not.toMatch('Error parsing tsconfig.json'); + expect(exitCode).toBe(0); + }); + + test('warns on invalid tsconfig.json in TS dependencies', async () => { + await using fixture = await createFixture({ + 'node_modules/fake-lib': { + 'package.json': JSON.stringify({ + name: 'fake-lib', + }), + 'tsconfig.json': tsconfigJson({ + extends: 'unresolvable-dep', }), 'index.ts': 'export function testFn(): string { return "Hi!" }', }, @@ -373,10 +433,9 @@ export default testSuite(({ describe }) => { const { stdout, exitCode } = await execa(webpackCli, { cwd: fixture.path, - reject: false, }); - expect(stdout).toMatch('Error parsing tsconfig.json:\nFile \'something-imaginary\' not found.'); + expect(stdout).toMatch('Error parsing tsconfig.json:\nFile \'unresolvable-dep\' not found.'); // Warning so doesn't fail expect(exitCode).toBe(0); From 6fb24a73e261b6916a92c7e822e0e884fa9a9393 Mon Sep 17 00:00:00 2001 From: Hiroki Osame Date: Tue, 16 Jul 2024 20:02:48 +0900 Subject: [PATCH 5/5] wip --- src/loader.ts | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/loader.ts b/src/loader.ts index 181ec3e..4cb70a7 100644 --- a/src/loader.ts +++ b/src/loader.ts @@ -15,6 +15,8 @@ import type { LoaderOptions } from './types.js'; const tsconfigCache = new Map(); +const tsExtensionsPattern = /\.(?:[cm]?ts|[tj]sx)$/; + async function ESBuildLoader( this: webpack.loader.LoaderContext, source: string, @@ -52,7 +54,7 @@ async function ESBuildLoader( // If file is local project, always try to apply tsconfig.json (e.g. allowJs) // If file is dependency, only apply tsconfig.json if .ts - && (!isDependency || resourcePath.endsWith('.ts')) + && (!isDependency || tsExtensionsPattern.test(resourcePath)) ) { /** * If a tsconfig.json path is specified, force apply it @@ -61,7 +63,7 @@ async function ESBuildLoader( * * However in this case, we also warn if it doesn't match */ - if (tsconfigPath) { + if (!isDependency && tsconfigPath) { const tsconfigFullPath = path.resolve(tsconfigPath); const cacheKey = `esbuild-loader:${tsconfigFullPath}`; let tsconfig = tsconfigCache.get(cacheKey); @@ -78,7 +80,7 @@ async function ESBuildLoader( if (!matches) { this.emitWarning( - new Error(`[esbuild-loader] The specified tsconfig at "${tsconfigFullPath}" was applied to the file "${resourcePath}" but does not match its "include" patterns`), + new Error(`esbuild-loader] The specified tsconfig at "${tsconfigFullPath}" was applied to the file "${resourcePath}" but does not match its "include" patterns`), ); }