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

types.ts files do not trigger rebuild in webpack-dev-server #36

Closed
abirmingham opened this issue Jul 20, 2017 · 58 comments
Closed

types.ts files do not trigger rebuild in webpack-dev-server #36

abirmingham opened this issue Jul 20, 2017 · 58 comments
Labels

Comments

@abirmingham
Copy link

First - thanks for the plugin and associated speed improvements!

One issue I've seen is that webpack-dev-server will not rebuild when a change is made in a source file that will not emit js. For example, if I have a types.js file that exports nothing but types/interfaces, and I make a change to that file that introduces a type error, I will not see a warning for that error until I change a file that does emit js. Here is an example:

  // types.ts
  export interface Person {
    name:string;
  };

  // mainEntry.ts
  import {Person} from 'types';
  const sally:Person = {name: 'Sally'};

Changes to mainEntry.ts will trigger a rebuild, but updating types.ts with name:number does not trigger a rebuild, and the error is unnoticed.

Relevant webpack config is as follows:

var threadLoader = {
    loader: 'thread-loader',
    options: {
        // leave one cpu for the fork-ts-plugin
        workers: require('os').cpus().length - 1,
    },
};

var babelLoader = {
    // XXX: .babelrc doesn't seem to be respected so we specify options here
    loader: 'babel-loader',
    options: {
        cacheDirectory: true,
        presets: ['es2015'],
    },
};

<snip>
    modules: {
        rules: [
            {
                test: tsSrc,
                exclude: babelExcludes,
                use: [
                    { loader: 'cache-loader' },
                    threadLoader,
                    babelLoader,
                    { loader: 'ts-loader', options: { happyPackMode: true } },
                ],
            },
<snip>
    plugins: [
        new ForkTsCheckerWebpackPlugin({
            tslint: true,
            watch: ['./src', './test', './test_util'], // optional but improves performance (less stat calls)
        }),
<snip>
@johnnyreilly
Copy link
Member

I've been working with @abirmingham and have seen this myself. Do you have any ideas @piotr-oles? Thanks!

@piotr-oles
Copy link
Collaborator

This plugin has no influence to the build watcher. You can comment out plugin and build should not trigger after .d.ts change. I suspect that issue is in ts-loader or combination of ts-loader and HappyPack. :)

@johnnyreilly
Copy link
Member

Hi @piotr-oles, thanks for responding! The issue is not phrased very clearly. Let me share my experience:

If tweaking a file which is interfaces only, fork-ts-checker-webpack-plugin does not seem to notice the change and perform checking again. It's like it only watches files that would produce JavaScript if TSC was pointed at them. Have you any ideas why this might be?

@johnnyreilly
Copy link
Member

To be clear: it's that fork-ts-checker-webpack-plugin does not seem to detect changes to interface only files. You'd imagine that it would, as a change to an interface could change a successful type check into a failed one. (Hope that makes sense)

@piotr-oles
Copy link
Collaborator

fork-ts-checker-webpack-plugin is hooked to the compilation run and if compilation doesn't run after file change, it will not run checker. I think it doesn't run because of some optimization in loader in transpileOnly mode - there is no need to recompile project if only typings have been changed.

Maybe it's bad design to rely on compilation run in async mode - maybe I should make it totally async... It can check files that are not a part of webpack's compilation so maybe it will be better behavior :)

@johnnyreilly
Copy link
Member

Thanks for the clarification @piotr-oles. It would be good if we could resolve this - I'm open to making changes in ts-loader if that would help. Though what you've suggested might be the better design choice...

In the meantime is it worth noting this down on the readme as a known issue for now? Happy to submit a docs PR....

I guess an ugly workaround which might / might not work would be to have some actual JavaScript in interface only files to ensure that file changes always trigger compilation. Sounds like a terrible hack. Not recommended...

@piotr-oles
Copy link
Collaborator

Always happy to see PR :)

@abirmingham
Copy link
Author

Thanks for the replies, @johnnyreilly and @piotr-oles!

I don't have much experience with webpack plugin development, but if I'm reading awesome-ts-loader correct, it appears that they hook into 'after-compile' here, and add dependencies in that function here. Should fork-ts-checker-webpack-plugin do the same? Apologies if I'm misinterpreting the issue!

@johnnyreilly
Copy link
Member

Completely by the by but for interest you can see the corresponding code in ts-loader here and here. That said, I think @piotr-oles is proposing not depending on ts-loader's compilation (I think)

@abirmingham
Copy link
Author

That makes sense. @piotr-oles if you have a suggested implementation I can take a shot at it and create a PR :)

@johnnyreilly
Copy link
Member

In the meantime is it worth noting this down on the readme as a known issue for now? Happy to submit a docs PR....

I've made the suggested docs PR now.

fork-ts-checker-webpack-plugin is hooked to the compilation run and if compilation doesn't run after file change, it will not run checker.

Like @abirmingham I'd be happy to help out on a suggested implementation to fix this problem. Do you have an idea that you'd like to try?

I haven't looked in depth at the reasons why this is happening but I'd speculate that this is to do with webpack seeing that the output from ts-loader hasn't changed (outputBefore === '' and outputAfter === '' ) so doesn't trigger compile. My knowledge of the plugin API is a little sketchy though and so I can't be sure. (This is useful by the way: https://webpack.js.org/api/plugins/)

@sokra - if you have a moment, is that a reasonable assumption of webpack behaviour on my part?

@piotr-oles
Copy link
Collaborator

piotr-oles commented Aug 1, 2017

There is an interesting proposal in TypeScript project (thanks @johnnyreilly for cc ;) ): microsoft/TypeScript#17493 so I think the best idea is to wait for it and implement when it will be ready :)
Another solution would be to just watch files from fork-ts-checker plugin but it would add another watcher :/ The hard part would be to make sure it watches all files it should and to be consistent with TypeScript or WebPack. The proposal I've mentioned makes WebPack, TypeScript and fork-ts-checker consistent.

@johnnyreilly
Copy link
Member

I'm glad you noticed @piotr-oles - I'm never too sure if everyone sees their cc's. 🌻

so I think the best idea is to wait for it and implement when it will be ready :)

I think you may be right; do feel free to pitch in on the comment thread. Your input will be very valuable to them and the TypeScript team are friendly and receptive people!

On another matter, from what @TheLarkInn has said about "deprecated accessing of tapable instances inside the loaders", I get the impression that transpileMode will become the only "approved" way to implement a typescript loader for webpack in the future. That is to say; ts-loader presently does 2 things:

  1. transpile
  2. check types and report them to webpack

And doing "2" involves hooking into webpack internals. I don't have the details but it sounds like the only way to do "2" in a webpack friendly way will be through moving type checking into a plugin (just as fork-ts-checker-webpack-plugin does) and only to run ts-loader in transpileMode. If that is the case then that has serious implications for the design of any typescript loaders.

My understanding may not be 100% on this and so I'd appreciate anyone who can set me straight doing so.

cc @jbrantly

@zinserjan
Copy link

The hard part would be to make sure it watches all files it should and to be consistent with TypeScript or WebPack. The proposal I've mentioned makes WebPack, TypeScript and fork-ts-checker consistent.

How does this makes all of them consistent? createWatchMode sounds for me like an additional watcher that runs independently of webpack? If this is the case, how can we synchronize them for non-async mode?

Btw this issue is already solvable when you pass all type checked files back to webpack, so webpack can detect changes on this file and trigger another build. But this will work only for non-async mode.

@code-elf
Copy link

With microsoft/TypeScript#17493 now being resolved, is there a chance of seeing a fix for this now?

@stnwk
Copy link

stnwk commented May 14, 2018

Would really love seeing a fix for this :) pushes politely

@piecyk
Copy link
Contributor

piecyk commented May 22, 2018

It looks like it's working it's not 😞

@abirmingham
Copy link
Author

@piecyk that's exciting, what version is it working on? Where/when was it fixed?

@piecyk
Copy link
Contributor

piecyk commented May 22, 2018

@abirmingham hard to say, just testing fork-ts with

"webpack": "4.8.3",
"typescript": "2.8.3",
"ts-loader": "4.3.0",
"fork-ts-checker-webpack-plugin": "^0.4.1",

Basic changing ts file with only type triggers rebuild, and correct type diagnostic to be presented.
Maybe @johnnyreilly has more insides ?

@johnnyreilly
Copy link
Member

If it's fixed that's awesome! I believe there's been a couple of PRs along the way that I have not been involved with. Maybe it was one of them that resolved it?

@a-tarasyuk
Copy link

a-tarasyuk commented May 22, 2018

@piecyk hm..., the same versions of packages., however, changing ts(x) files which have only types definitions does not trigger a rebuild.

@piecyk
Copy link
Contributor

piecyk commented May 22, 2018

@johnnyreilly @a-tarasyuk @abirmingham false alarm Guys, it's not working, tested on repo from scratch... It looks like on my more complex repo that mislead me something is triggering the watch to run...

@janwirth
Copy link

Bump :)

@piotr-oles piotr-oles mentioned this issue Apr 18, 2020
26 tasks
@piotr-oles
Copy link
Collaborator

Thank you @oviava for the comment about importsNotUsedAsValues: preserve - I documented it in the README.md in the new alpha release :) I will close this issue as this configuration solves the problem :)

@joaoreynolds
Copy link

This flag doesn't seem to help me :(

.babelrc.js

module.exports = api => {
  const isTest = api.env('test')
  return {
    "presets": [
      "@babel/preset-typescript",
      "@babel/preset-react",
      [
        "@babel/preset-env",
        isTest ? { // babel needs to target node for jest tests instead of what's in the browserslist in package.json
          "useBuiltIns": "entry",
          "corejs": "3",
          "targets": {
            "node": "current"
          }
        } : {
          "useBuiltIns": "entry",
          "corejs": "3"
        }
      ]
    ],
    "plugins": [
      "@babel/plugin-proposal-class-properties",
      "babel-plugin-styled-components"
    ]
  }
}

tsconfig.json

{
  "compilerOptions": {
    "baseUrl": "src",
    "outDir": "./dist/",
    "noImplicitAny": false,
    "module": "es6",
    "moduleResolution": "Node",
    "target": "es5",
    "jsx": "react",
    "allowJs": true,
    "sourceMap": true,
    "allowSyntheticDefaultImports": true,
    "esModuleInterop": true,
    "noEmit": true,
    "importsNotUsedAsValues": "preserve"
  }
}

webpack.config.js

...
new ForkTsCheckerWebpackPlugin({
      typescript: {
        diagnosticOptions: {
          semantic: true,
          syntactic: true,
        },
      },
    }),
...

@johnnyreilly
Copy link
Member

I reopened this as @sangeeth96 opened an issue which seems to be this. It could be worth trying the importsNotUsedAsValues: preserve tip anyone that comes here - but perhaps it doesn't solve all issues?

@piotr-oles
Copy link
Collaborator

It seems like an issue with the babel-loader - it works with ts-loader as far as I know (we have e2e tests for it - but only for ts-loader). I have to check some ideas on why it might not work.

@johnnyreilly
Copy link
Member

Thanks @piotr-oles - in one of those "no buses come and then 3 arrive at once" things we've had a second person raise this issue. I've closed it as a duplicate and linked to this

@dmarkow
Copy link

dmarkow commented Jul 14, 2020

Same issue here. I use graphql-code-generator to create an api-types.d.ts file for my API types. Since v5, any time that d.ts file gets updated, I have to completely restart webpack to get it to recognize the updated types. I've tried setting importsNotUsedAsValues to preserve but it didn't help. In v4, updates to my d.ts files always worked fine.

@dmarkow
Copy link

dmarkow commented Jul 14, 2020

A little more info: I'm using babel-loader and it's definitely skipping over the d.ts files since they only include types. That's unrelated to the v4->v5 upgrade for this plugin.

In both v4 and v5, updating a d.ts wouldn't trigger a rebuild. However, I could go to the .tsx file that was importing the d.ts file, re-save it, and webpack would then rebuild successfully, with this plugin recognizing the updated types.

In v5, re-saving the .tsx file triggers a rebuild, but the rebuild fails with a type error (for example, if I add property bar to type Foo and try to reference it, it'll still say TS2339: Property 'bar' does not exist on type 'Foo' until I fully restart webpack). So it seems that the d.ts file isn't being re-read even though it has been changed; instead, it's using a cached version from a previous run.

@Sayvai
Copy link

Sayvai commented Sep 17, 2020

Current relevant dependencies:

  • fork-ts-checker-webpack-plugin @ 5.1.0
  • babel-loader @ 8.0.6
  • ts-loader @ 4.5.0

Checking up on whether there is any progress on this issue?

Webpack is not recompiling when a type (e.g, interface) is modified and then saved.

This occurred after upgrading fork-ts-checker-webpack-plugin from version 4 to 5+.

@RistiCore
Copy link

The same problem:

  • fork-ts-checker-webpack-plugin 5.1.0
  • babel-loader 7.1.5
  • ts-loader 8.0.3

Rolling fork-ts-checker-webpack-plugin back to 4.1.6 resolves the problem.

@piotr-oles
Copy link
Collaborator

I'm working on a fix, there is progress :) Unfortunately, I'm not able to give you time frame when it will be fixed

@piotr-oles
Copy link
Collaborator

I've released a new version on the alpha channel - ’6.0.0-alpha.1’. It should detect changes in type-only files without additional TypeScript/Babel configuration :)

@johnnyreilly
Copy link
Member

Nice work!

@bhollis
Copy link

bhollis commented Oct 20, 2020

I'm not sure if this case is unique, but we use css-modules-typescript-loader to generate typings (.d.ts files) for CSS modules. Using [email protected] we still see an issue where types from these generated files don't get picked up until webpack is restarted.

@piotr-oles
Copy link
Collaborator

@bhollis Would you be able to create a small reproduction repository?

@bhollis
Copy link

bhollis commented Oct 23, 2020

Here you go! I hope it's minimal enough: https://github.com/bhollis/fork-ts-checker-repro

@piotr-oles
Copy link
Collaborator

@bhollis Your reproduction repo uses fork-ts-checker-webpack-plugin@^5.0.5 - switching to ^6.0.0-alpha.1 resolves the issue :)

@bhollis
Copy link

bhollis commented Oct 23, 2020

Fantastic! For future reference, what was breaking it for us was this line, which was once recommended as part of the css-modules-typescript-loader instructions:

new webpack.WatchIgnorePlugin([/scss\.d\.ts$/]),

That was disabling Webpack's watcher which meant your fix wasn't able to run.

@piotr-oles
Copy link
Collaborator

I'm closing this as it seems to be resolved with the 6.0.0 release :)

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

No branches or pull requests