Skip to content

Support for create-react-app, typed css-modules and some other stuff... #25

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

Closed
zinserjan opened this issue Jun 29, 2017 · 5 comments
Closed

Comments

@zinserjan
Copy link

Thank you for your work on this plugin. This approach improves webpack build times immense, especially incremental times are a lot faster (even in blocking mode).

But there are some issues at the moment that keeps us from using this plugin in the typescript fork of CRA:

  • All type checking errors/warnings needs to be forwarded to webpack’s compile lifecycle, cause CRA controls the terminal output (auto clear) and shows errors in the browser.
    -> watch mode needs to block webpack compilation until type checking is done
  • File watcher should be configured by default to get the performance improvements
  • Displayed type errors should look similar to ts-loader

Additionally to these, I would also like to see support for typed css modules. The main issue here is that the type checking can only be done after the css files were processed cause type definitions for css files will be created on the fly.

I tried to fork this plugin and fix these „issues“. Adding the blocking of the emit phase back in watch mode was quite easy, but adding support for typed css modules was more complex. That’s why I started to work on a POC for this and ended up with a new plugin, written in TypeScript and tested by Jest.

The approach is the same like your plugin, but there are some differences:

  1. The type checking starts after all files were built by webpack (seal hook), but the compilation still needs to be packed together and some other magic needs to be done to optimize the bundle.
  2. All files are cached by default until they will be invalidated by webpack. This is possible cause we have the information which files were (re-)built, due to first point.
  3. Webpack’s built in file watcher is used to watch additional files like type definitions. So theres no need for a secondary file watcher with it’s own configuration.
  4. Only files that are used by webpack will be type checked & linted. This is also possible cause of 1.
  5. Compilation is always blocked until type checking is done. In my test project type checking was in the most cases faster than webpack on incremental builds.
  6. There is no support of type checking cancellation, cause hacks like checking for file existence are too expensive due to the I/O usage (didn’t checked if this was really called every 10ms). This is also superfluous due to the blocking.
  7. There are different error formatters for type checking errors (like ts-loader, code frame, …)
  8. There is only a single thread for type checking. In my test cases I never saw a benefit on incremental times with more than 1 thread. Question is how many modules needs to be checked to see a benefit for multiple processes here? And the other question is, can we configure this automatically so that the user just defines a maximal number of processes and we optimize the used processes under the hood.
  9. it works great with typed CSS-Modules :)

I made some performance comparisons with your plugin (modified to block ) vs. my plugin and your plugin was on the initial built around 10% faster but on incremental builds around 30% slower. But this was on a quite small project. Unfortunately I don’t have a bigger project to test the impact of the blocking mode, the lack of multiple processes and the delayed start of type checking.

@piotr-oles
It would be awesome if you can provide some statics when the multi process starts to make sense. And it would be even more great, when you could find the time to compare it with my plugin. I pushed my changes for the blocking mode to my fork of this plugin.

Regardless of the results, I think it would be good to join forces to get something that works for everyone and is also as fast as possible :)

@johnnyreilly
Copy link
Member

Working together always a good thing 💟

@piotr-oles
Copy link
Collaborator

Thank you for your work, I really appreciate that :)
I did tests with your plugin on pretty big project (~103 000 LOC)

The results are:

  • Initial:

    • fork-ts-checker-webpack-plugin: ~70 seconds (~45 seconds for check)
    • ts-checker-webpack-plugin: ~85 seconds
  • Incremental:

    • fork-ts-checker-webpack-plugin: ~10 seconds (~17 seconds for check)
    • ts-checker-webpack-plugin: ~30 seconds

We use 2 processes and it gives us ~4 seconds speed-up on incremental build

I think your plugin will be better for medium-size projects, mine for bigger projects :)

I very like customization of formatters - it would be good to add this to fork-ts-checker-webpack-plugin.

Also we could add blockMode in plugin options to force block on wath mode.

@johnnyreilly
Copy link
Member

johnnyreilly commented Sep 8, 2017

@zinserjan would you be up for forking and sending a PR with the error / warning formatting changes in? Since you raised this initial issue the project has moved on somewhat; it's now written in TypeScript and I think the async option may cover your blocking concerns?

As @piotr-oles says:

I very like customization of formatters - it would be good to add this to fork-ts-checker-webpack-plugin.

So it would be good to get some of your work contributed to the plugin where we there is common ground 🌷

@zinserjan
Copy link
Author

it's now written in TypeScript

Yeah, good job!

would you be up for forking and sending a PR with the error / warning formatting changes in?

Whats wrong with #31? (Never tried it)

I think the async option may cover your blocking concerns?

Yep, but #36 needs to be resolved before this is usable for me. This works already with my plugin.
Furthermore #40 needs to be documented.

@piotr-oles
Copy link
Collaborator

I close the issue due to no activity :)

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

No branches or pull requests

3 participants