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

Webpack 5 cache - typings not cached #1332

Open
webpack-bot opened this issue May 28, 2021 · 36 comments
Open

Webpack 5 cache - typings not cached #1332

webpack-bot opened this issue May 28, 2021 · 36 comments

Comments

@webpack-bot
Copy link

Bug report

What is the current behavior?
d.ts files are not emitted after removing the output folder and running webpack again.
I use ts-loader and have set declaration: true and outDir: 'dist' in tsconfig.json.

If the current behavior is a bug, please provide the steps to reproduce.

Run webpack build for the first time - bundle.js and typings are emitted. Remove 'dist' folder and run webpack again.

webpack.config.js:

{
  mode: isDevelopment ? 'development' : 'production',
  entry: {
    main: ['./src/index.tsx'],
  },
  output: {
    filename: 'bundle.js',
    path: path.resolve(__dirname, 'dist'),
    publicPath: '/',
  },
  cache: {
    type: 'filesystem',
    buildDependencies: {
      sources: [path.resolve(process.cwd(), 'src') + '/'],
      tsConfig: [path.resolve(process.cwd(), 'tsconfig.json')],
      config: [__filename]
    },
  },
  module: {
    rules: [
      {
        test: /\.tsx?$/,
        include: path.join(__dirname, 'src'),
        use: {
          loader: 'ts-loader'
        }
      },
      {
        test: /\.jsx?$/,
        include: path.join(__dirname, 'src'),
        use: 'babel-loader',
      },
    ],
  },
  resolve: {
    extensions: ['.ts', '.tsx', '.js', '.jsx'],
  },
};

What is the expected behavior?
I expect idempotent behavior - set of files emitted by webpack build is the same regardless if the cache is utilized or not.

Other relevant information:
webpack version: 5.37.1
Node.js version: 14.15.4
Operating System: Ubuntu 20.04
Additional tools: ts-loader: 9.2.2, typescript: 4.0.3


This issue was moved from webpack/webpack#13465 by @alexander-akait. Original issue was by @MBelniak.

@alexander-akait
Copy link
Contributor

@johnnyreilly I think it will be great thing, cache will help with big projects very good, do you need help? I want to look at this in the next week (example usage of cache here https://github.com/webpack-contrib/terser-webpack-plugin/blob/master/src/index.js#L214, do not look at logic for extract comments)

@johnnyreilly
Copy link
Member

If you'd like to look at this I'd be happy to look at the PR!

@MBelniak
Copy link

@alexander-akait @johnnyreilly Hi guys, any update on this?

@johnnyreilly
Copy link
Member

I'm not working on this, but I'm happy to review PRs

@stale
Copy link

stale bot commented Apr 17, 2022

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the wontfix label Apr 17, 2022
@stale
Copy link

stale bot commented Apr 28, 2022

Closing as stale. Please reopen if you'd like to work on this further.

@stale stale bot closed this as completed Apr 28, 2022
@vsternbach
Copy link

As a workaround, it is possible to use fork-ts-checker-webpack-plugin alongside ts-loader and it supports typings caching

@yslpn
Copy link

yslpn commented Sep 5, 2022

Is there any news in this direction?

@alexander-akait
Copy link
Contributor

@johnnyreilly Maybe will be great to reopen, because it can be still useful

@johnnyreilly johnnyreilly reopened this Sep 5, 2022
@stale stale bot removed the wontfix label Sep 5, 2022
@johnnyreilly
Copy link
Member

Reopened - if anyone would like to work on it that's great. I'm not planning to myself, so we'll need someone to step up if they want this to progress

@a-coruble
Copy link

Hey there, in my company we're using TS-Loader in multiple Webpack packages, including some in a monorepo, and we wanted to implement Webpack Cache, but faced this issue where we hit the cache but don't get the typings as they're not part of Webpack's tracked output files. I would be really interested in contributing but would need heavy guidance at first as I don't know Webpack's source code / plugin API.

@johnnyreilly
Copy link
Member

@alexander-akait would you be able to advise?

@alexander-akait
Copy link
Contributor

@johnnyreilly Yeah, can you provide a link on code where we generate them?

@johnnyreilly
Copy link
Member

I don't know where they are generated - I thought you might!

@a-coruble
Copy link

Any hints by where I could start learning / deep-diving in your codebase even if it's not the link to the part responsible for type generation. I would really like to contribute here 😄

@alexander-akait
Copy link
Contributor

@johnnyreilly I mean where ts-loader generate types and emit them 😄 We need to add cache logic here, so I want to look how it is implemented right now and I will gave example/good advice how we can improve it, ideally what I want to see

  • Input (how and where it is)
  • Output (how and where it is)

@a-coruble
Copy link

a-coruble commented Apr 19, 2023

@johnnyreilly @alexander-akait any news on that side? I tried to find where types get generated in the loader, but didn't find it by myself 😅

@johnnyreilly
Copy link
Member

I think that this is where types are supplied to webpack (output):

provideDeclarationFilesToWebpack(

And likewise any other references to the same method I think.

I think input will be the same as any other webpack loader - webpack supplies the file contents for processing

@alexander-akait
Copy link
Contributor

Thank you, I will investigate it soon (today/tomorrow) and will send a PR (or ask more infromation)

@a-coruble
Copy link

Hey there, allowing myself to ask for an update on this, or as usual, some guidance to implement it myself?

@alexander-akait
Copy link
Contributor

alexander-akait commented May 16, 2023

@johnnyreilly Okay, I looked at code and found that typescript emits these files itself, not ts-loader, what we can:

  • add these files to dependecies, but if you remove them ts-loader will run again, so we won't get any advantage here
  • say typescript doesn't emit them and emit them itself (in ts-loader), but it is tricky, because logic is deeply inside typescript (maybe you know how to achive it, I am not good in typescript API)
  • say, it is a limitation, but it mean if you enable output.clean you will lose these files.

Another big question - why developers have "declaration": true, "outDir": "dist", for ts-loader, may I am missing something 😕 , but that is a point to do it? When you build library you have typescript declarations and compiled files to JS (by typescript, no webpack here), when you build an application you don't need d.ts files inside the dist directory (why have them?).

@alexander-akait
Copy link
Contributor

alexander-akait commented May 16, 2023

We have this https://github.com/TypeStrong/ts-loader#declaration-files-dts

To output declaration files (.d.ts), you can set "declaration": true in your tsconfig and set "transpileOnly" to false.

If you use ts-loader with "transpileOnly": true along with fork-ts-checker-webpack-plugin, you will need to configure fork-ts-checker-webpack-plugin to output definition files, you can learn more on the plugin's documentation page: https://github.com/TypeStrong/fork-ts-checker-webpack-plugin#typescript-options

So I think the second point is more valid than the first, I will continue to looking for a way how to solve it, but not sure it is really possible

@alexander-akait
Copy link
Contributor

alexander-akait commented May 16, 2023

Anyway I found a place the problem - https://github.com/TypeStrong/ts-loader/blob/main/src/after-compile.ts#L29, shorty - when cache enabled, loader won't start (because why? it is only wasting time), so hooks will not registered and so nothing will be emit.

@johnnyreilly Why we have a such design? It is not valid accroding webpack logic, everything inside loader should be only inside loader, we have this.emitAsset and this.emitError and other methods to achive it. We need to remove it, i.e. rewrite to build-in loader API this and cache will work out of box without any problems

@alexander-akait
Copy link
Contributor

I tried to rewrite it, but some of tests are failed, so I need a developer who written it and we need to rewrite it, also it allows to improve perf very well when cache is enabled

@alexander-akait
Copy link
Contributor

alexander-akait commented May 16, 2023

The same asnwer vercel/ncc#208 (comment) and this issue can be union with #894, I am afraid without @johnnyreilly your help I can't rewrite it without problems and regressions

@alexander-akait
Copy link
Contributor

alexander-akait commented May 16, 2023

Also possible improvements:

  • use webpack built-in logger, so everything can be in stats
  • use a new API for resolver, so developers don't need to add this:
resolve: {
    // Add `.ts` and `.tsx` as a resolvable extension.
    extensions: [".ts", ".tsx", ".js"],
    // Add support for TypeScripts fully qualified ESM imports.
    extensionAlias: {
     ".js": [".js", ".ts"],
     ".cjs": [".cjs", ".cts"],
     ".mjs": [".mjs", ".mts"]
    }
  },
  • also it allows to custmozire resolver logic on module level

@johnnyreilly
Copy link
Member

@johnnyreilly Why we have a such design? It is not valid accroding webpack logic, everything inside loader should be only inside loader, we have this.emitAsset and this.emitError and other methods to achive it.

To be honest, I can't remember anymore - ts-loader has been in my hands for 7 years and this is information I've now forgotten

The nature of our test pack is that it's quite brittle - certainly the comparison test part

I'm happy to review a rewrite, but I don't plan to do one myself.

@alexander-akait
Copy link
Contributor

@johnnyreilly at the very least we need to research and provide a roadmap to rewrite it (step by step) otherwise no one will be able to solve it and we will accumulate problemsm without you I can't do it, because I don't know why this code exists. We can start with easy - just add comments to each functions https://github.com/TypeStrong/ts-loader/blob/main/src/after-compile.ts#L36 here

@johnnyreilly
Copy link
Member

We can start with easy - just add comments to each functions https://github.com/TypeStrong/ts-loader/blob/main/src/after-compile.ts#L36 here

I'm not sure I know what you mean. Which functions? What comments would you like?

@alexander-akait
Copy link
Contributor

@johnnyreilly I mean like https://github.com/TypeStrong/ts-loader/blob/main/src/after-compile.ts#L50, https://github.com/TypeStrong/ts-loader/blob/main/src/after-compile.ts#L67 and https://github.com/TypeStrong/ts-loader/blob/main/src/after-compile.ts#L74 and etc, shortly - we need to rewrite all of them and put logic inside loader, not in the hook, so I need to undestand why it here and what it is doing

@johnnyreilly
Copy link
Member

If memory serves, afterCompile is mostly used for type checking. ts-loader operates in 2 modes: with type checking (transpileOnly: false) and without (transpileOnly: true). Type checking cannot operate cannot operate in the loader context as it's the result of multiple files, not a single file; that's why it's in the hook. So I don't think that this type checking (and related) logic can move to the loader instead of the hook.

@alexander-akait
Copy link
Contributor

@johnnyreilly But here vercel/ncc#208 (comment) you said another...

@alexander-akait
Copy link
Contributor

alexander-akait commented May 23, 2023

@johnnyreilly Okay, let's split it in simple tasks, what we need firstly - move logic of this https://github.com/TypeStrong/ts-loader/blob/main/src/after-compile.ts#L80 inside loader, is any history or problems why it was moved to here, because when we build module we already have such files and can emit them inside loader, so cache will work (errors reporting will not be cachable, but it is no a big problems, because in good application no warnings and errors 😄 )

@johnnyreilly
Copy link
Member

I can't remember the rationale - it's been quite a while. If you'd like to work on a PR to move it, I'm happy to take a look!

@a-coruble
Copy link

@alexander-akait Hey there, I'd be happy to pair / help now that we have a better idea of what needs to be done in order to get typings cached! Let me know how I can help!

@alexander-akait
Copy link
Contributor

@a-coruble Briefly - we need to move this function https://github.com/TypeStrong/ts-loader/blob/main/src/after-compile.ts#L44 (i.e. we should not emit files in the plugin) to here https://github.com/TypeStrong/ts-loader/blob/main/src/index.ts#L49 (i.e. emit them on loader level), I didn't look at this deeply, but I think it should works without any problems, feel free to try it and send a PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants