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

upgrading from v3 to v4 breaks build for TS projects #343

Open
2 tasks
vritant24 opened this issue Oct 26, 2023 · 11 comments
Open
2 tasks

upgrading from v3 to v4 breaks build for TS projects #343

vritant24 opened this issue Oct 26, 2023 · 11 comments
Labels
bug Something isn't working pending triage pr welcome

Comments

@vritant24
Copy link

Problem

I've created the following repo for reproducing the issue: https://github.com/vritant24/webpack-esbuild-loader-4-repro

Steps:

  1. run npm install
  2. run npm run build

The build should succeed.

If you upgrade the esbuild-loader version in package.json to 4.0.2, you will see errors similar to:

 WARNING in ./src/common/lib.ts
  Module Warning (from ./node_modules/esbuild-loader/dist/index.cjs):
  [esbuild-loader] The specified tsconfig at "C:\repos\webpack-esbuild-loader-4-repro\tsconfig.json" was applied to the file "C:\repos\webpack-esbuild-loader-4-repro\src\common\lib.ts" but does not match its "include" patterns
  Error: [esbuild-loader] The specified tsconfig at "C:\repos\webpack-esbuild-loader-4-repro\tsconfig.json" was applied to the file "C:\repos\webpack-esbuild-loader-4-repro\src\common\lib.ts" but does not match its "include" patterns
      at Object.ESBuildLoader (C:\repos\webpack-esbuild-loader-4-repro\node_modules\esbuild-loader\dist\index.cjs:60:11)
   @ ./src/desktop/main.ts 1:0-37 2:0-4

Expected behavior

Build should succeed

Minimal reproduction URL

https://github.com/vritant24/webpack-esbuild-loader-4-repro

Version

4.0.2

Node.js version

v16

Package manager

npm

Operating system

Windows

Contributions

  • I plan to open a pull request for this issue
  • I plan to make a financial contribution to this project
@vritant24 vritant24 added bug Something isn't working pending triage labels Oct 26, 2023
@privatenumber
Copy link
Owner

@vritant24
Copy link
Author

vritant24 commented Oct 27, 2023

Thanks for the quick reply!

Apologies I accidentally made private repo. Fixed that. I hope the repo elaborates my issue better

  • the tsconfig is included, and the files are included as part of referenced tsconfigs
  • There is a Warning, but it also comes as an error underneath, which fails our build:
WARNING in ./src/desktop/main.ts
  Module Warning (from ./node_modules/esbuild-loader/dist/index.cjs):
  [esbuild-loader] The specified tsconfig at "C:\repos\webpack-esbuild-loader-4-repro\tsconfig.json" was applied to the file "C:\repos\webpack-esbuild-loader-4-repro\src\desktop\main.ts" but does not match its "include" patterns
  Error: [esbuild-loader] The specified tsconfig at "C:\repos\webpack-esbuild-loader-4-repro\tsconfig.json" was applied to the file "C:\repos\webpack-esbuild-loader-4-repro\src\desktop\main.ts" but does not match its "include" patterns
      at Object.ESBuildLoader (C:\repos\webpack-esbuild-loader-4-repro\node_modules\esbuild-loader\dist\index.cjs:60:11)

@balasphilip

This comment was marked as spam.

@davidmurdoch

This comment was marked as off-topic.

@privatenumber
Copy link
Owner

Feel free to open a PR

@davidmurdoch

This comment was marked as off-topic.

@privatenumber

This comment was marked as off-topic.

@dons20
Copy link

dons20 commented Dec 5, 2023

So after much testing, I think I understand a little bit more of how this is happening.

If esbuild-loader is involved in transpilation of a file that happens to import something that your tsconfig.json did not include in its options, it will throw this warning.

E.g. If you have a .tsx file that imports an svg that you are later processing with another loader (@svgr/webpack for example), it will read and process that svg import and will try to apply the tsconfig.json in your project to that particular file as well.

This may be a good warning for the sake of clarity, but it's also a little bit unexpected in my opinion. This was added in V4. https://github.com/privatenumber/esbuild-loader/releases/tag/v4.0.0

To immediately fix this, you can add the tsConfigRaw: {} to the loader options though be careful if you need specific TS options added. Here's an example following my situation

// Checks for SVG imports inside js(x) files
{
  test: /\.svg$/,
  exclude: /node_modules/,
  issuer: {
    test: /\.tsx?$/,
  },
  use: [
    {
      loader: 'esbuild-loader',
      options: {
        loader: 'tsx',
        tsconfigRaw: {},
      },
    },
    {
      loader: '@svgr/webpack',
      options: { ... },
    },
    {
      loader: 'url-loader',
      options: { ... },
    },
  ],
},

Also here are two suggestions I'd propose. If @privatenumber agrees, I wouldn't mind contributing a PR to modify some things too.

  1. Adding a separate config flag to hide these warnings. This warning can break builds / pipelines that don't allow warnings.
  2. If the v4 functionality was added to handle cases of multiple tsconfigs, perhaps the newer logic could apply when there are multiple tsConfigs? Perhaps this is something that can be changed via a flag as well?

@privatenumber
Copy link
Owner

@dons20

I appreciate you contributing constructively!

Here are the problems I see:

I'm open to the following PRs:

  • Test that emits this warning and shows that it fails. And another test that confirms Webpack configuration can suppress this warning.
  • Fix for the tsconfig.json matching bug and test

@Hotell
Copy link

Hotell commented May 6, 2024

@privatenumber thanks for creating this plugin !

we ran into the same issue.

The warning is actually inaccurate (TypeScript incompatibility)

I'm wondering how does this relate to typescript incompatibility ? AFAIK the only source of truth for loading module tree within webpack is entry field, thus anything specified within tsconfig#includes etc should be ignored ( unless esbuild provides type-checking features which will probably never happen ).

From my POV to resolve this processing issue of esbuild-loader any includes/files/exclude patterns within provided tsconfigs should be avoided and only module transpilation configuration within compilerOptions should be processed ( following esbuild ).

note:

Also as you realised regarding program creation within TS, anything that is being part of Program is included in both type-checking and transpiling even if it's not specified within include/files pattern , even if it's within exclude patterns and it is part of your program it will be consumed. This behaviour is in TS since day 1 AFAIR.

@privatenumber
Copy link
Owner

Yeah, I think I agree with you.

It doesn't matter too much now... but originally, I was trying to handle cases with multiple tsconfigs. For example, a tsconfig for the src directory, and another for the tests directory. So initially, I wanted to be careful the detected or specified tsconfig isn't applied to files outside of it's target scope.

But I see now I was misunderstanding TS behavior: TypeScript simply applies the tsconfig to all imported files even if only the entry-file is in includes/files/excludes. I think the only validation check we needs to enforce is verifying that the entry-file matches the detected/passed-in tsconfig file.

Another interesting case is a monorepo where one packages bundles another with a different tsconfig. This is a problem we have in https://github.com/privatenumber/tsx too.

Anyway, would you be willing to work on this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working pending triage pr welcome
Projects
None yet
Development

No branches or pull requests

6 participants