dojo / cli-build-webpack

🚀 **DEPRECATED** Dojo 2 - cli command for building applications
http://dojo.io
Other
4 stars 19 forks source link

Prevent css.d.ts files from triggering builds. #157

Closed mwistrand closed 7 years ago

mwistrand commented 7 years ago

Type: bug

The following has been addressed in the PR:

Description:

The css-module-dts-loader creates new d.ts files for each CSS file, but each new d.ts triggers a compilation. This PR utilizes the WatchIgnorePlugin to prevent files ending in .css.d.ts from triggering builds. There may be a better way to solve this problem, but setting watchOptions.ignored or exclude on the relevant loaders has no effect. This fix has been verified using the reproduction steps listed in the issue.

Resolves #137

matt-gadd commented 7 years ago

this does subtly change/break some of the expected behaviour I believe, here's an example:

  1. Run dojo build -w
  2. Remove a class from an m.css file that is being used somewhere in an actual widget
  3. No error reported from build until another file is written to.

Presumably the inverse is true also (adding a class that is used in a widget should result in a valid build when the m.css is written)

mwistrand commented 7 years ago

Sigh. I knew using WatchIgnorePlugin was too simple to be correct ;)

Webpack uses webpack/Watchpack to track changes to the file system; when new files are created, the watcher for some reason sets the modification timestamp 10s in the future, and then emits change events (therefore triggering rebuilds) until the compilation start time passes that arbitrary mtime. webpack/watchpack#51 has been reported, but in the meantime I might be able to fix this with a temporary hack.

mwistrand commented 7 years ago

I created a new IgnoreUnmodifiedPlugin that runs in watch mode as a temporary workaround for the aforementioned watchpack issue. It works by overriding the file system watcher's change handler, executing the original only when the file's mtime differs from the previous value.

I tested this as thoroughly as possible, confirming:

1) The app compiles only twice when $ dojo build --watch is first executed: first to compile the app, then again in response to the added/updated .css.d.ts files. 2) Removing a class from a CSS module results in a failed build when that class is used elsewhere. 3) Readding a removed class to a CSS module results in a successful build. 4) It is still possible to add new modules, either in the same directory or in a new directory, and have them added to the build when pulled into another module.

@matt-gadd If you can think of any scenario I have missed, please let me know.

agubler commented 7 years ago

@mwistrand This bug seems to only be an issue for the first build during watch, which the main use-case would be that the d.ts files already exist and do not need to be updated.

In my head we shouldn't be recreating the d.ts file unnecessarily and if we didn't it would mean for the majority of the use cases the watch would work as expected (on the first build and subsequent ones).

Perhaps an initial fix could be to prevent the build creating d.ts files unless the css file has a later mtime? It would mean we could potentially remove the workaround that overrides webpack watcher internals?

Thoughts?

agubler commented 7 years ago

What's the current status of this? Feels like this is something that we'd want to try and fix sooner rather than later.