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

Performance issues with watch mode #2

Closed
pmwmedia opened this issue Dec 14, 2022 · 12 comments · Fixed by #3
Closed

Performance issues with watch mode #2

pmwmedia opened this issue Dec 14, 2022 · 12 comments · Fixed by #3

Comments

@pmwmedia
Copy link

pmwmedia commented Dec 14, 2022

When using this plugin and having watch mode enabled in my Webpack configuration, I have serious performance issues. Without the plugin, Webpack needs just a few milliseconds for incremental runs. However, with the plugin, Webpack needs more than 10 seconds for incremental runs for my project.

OS: Windows 10
Webpack: 5.75.0
bundle-declarations-webpack-plugin: 3.1.1

Is there anything we can do? Currently, I disable bundle-declarations-webpack-plugin completely when watch mode is enabled as a workaround.

@dominicbirch
Copy link
Owner

I will do some investigation, there may be some optimisations I can make; it's likely that the .d.ts plugin which is being wrapped here is not getting the benefit of the caching + incremental behaviour from webpack and/or typescript.

Disabling the declarations bundle output in watch mode for now is perfectly ok unless you are working closely with the .d.ts output, but I understand that 10seconds is an unreasonable time to wait when this is the case, will see what I can do :)

@dominicbirch
Copy link
Owner

When using this plugin and having watch mode enabled in my Webpack configuration, I have serious performance issues. Without the plugin, Webpack needs just a few milliseconds for incremental runs. However, with the plugin, Webpack needs more than 10 seconds for incremental runs for my project.

OS: Windows 10 Webpack: 5.75.0 bundle-declarations-webpack-plugin: 3.1.1

Is there anything we can do? Currently, I disable bundle-declarations-webpack-plugin completely when watch mode is enabled as a workaround.

So just to properly understand, is the problem that it blocks your watch build from completing for this time, or that it takes 10 seconds to receive the .d.ts output?

If the problem is that it's impacting other aspects of the build we could consider running a separate parallel build for it, or running a background worker while in watch mode (similar idea to what fork-ts-checker-webpack-plugin does for typechecking in the background during watch mode), would this work as a resolution?

@dominicbirch
Copy link
Owner

see: timocov/dts-bundle-generator#190

The bundling also seems to have slowed down a bit; will see what I can do to mitigate this for webpack watch temporarily.

@pmwmedia
Copy link
Author

pmwmedia commented Dec 19, 2022

Currently, it blocks the entire watch build. I think a separate parallel build or a background worker might help.

Nevertheless, I need the output of the declaration bundle for other modules within the same workspace. However, the declarations don't change that often and I can accept if they are updated with a delay as long as the rest of the build is not blocked.

@dominicbirch
Copy link
Owner

Latest version of the library I'm wrapping does seem a little slow (taking about 12seconds to build this small library); I will look into this separately but will also try to take the .d.ts out of watchRun compilations so that it works out of process with a delay.

To be honest, I don't remember the dts-bundle-generator taking as long as this the last time I worked with it so not sure what's up, should really be at least comparable to the transpile & check.

@pmwmedia
Copy link
Author

I haven't measured it. However, I also think it was quite faster in the past.

@dominicbirch
Copy link
Owner

@pmwmedia give 4.0.0 a try when you can - couldn't figure out how to add a preview version on the npm registry, but there are quite a lot of changes in this one #3, hopefully all ok, let me know if not! 😅

@pmwmedia
Copy link
Author

pmwmedia commented Dec 23, 2022

Thank you for your quick and promising solution! I have just tested it. There seems to be an issue with the file extension. After updating to version 4.0.0, I receive an error when starting Webpack with watch mode:

ReferenceError [Error]: require is not defined in ES module scope, you can use import instead
This file is being treated as an ES module because it has a '.js' file extension and '[...]\node_modules\bundle-declarations-webpack-plugin\package.json' contains "type": "module". To treat it as a 
CommonJS script, rename it to use the '.cjs' file extension.
    at file:///[...]/node_modules/bundle-declarations-webpack-plugin/dist/worker.5925002d.js:1:7
    at ModuleJob.run (node:internal/modules/esm/module_job:193:25)
    at async Promise.all (index 0)
    at async ESMLoader.import (node:internal/modules/esm/loader:530:24)
    at async loadESM (node:internal/process/esm_loader:91:5)
    at async handleMainPromise (node:internal/modules/run_main:65:12)

By the way, I have a webpack.config.ts in Typescript.

@pmwmedia
Copy link
Author

When renaming worker.5925002d.js into worker.5925002d.cjs, I don't get the error anymore. However, I'm not sure if it is desired to have a worker.5925002d.cjswith requires and a worker.a442d1b4.js with imports.

However, I run in another issue now. The generated .d.ts file is now totally empty, regardless if I use watch mode or not. Therefore, it's pretty fast now!

@dominicbirch
Copy link
Owner

dominicbirch commented Dec 23, 2022

Interesting... I will add some tests around this and see if I can reproduce it; the idea is that the usage remains the same, but it will detect watch mode and use the worker in this instance. Both ESM & CJS should be supported, but glancing at it there might be an issue with both ESM & CJS for the worker chunk having a js extension (this might actually be a parcel bug, will look into it, would have expected the CommonJS to use .cjs)

@dominicbirch
Copy link
Owner

Should find it works in version 4.0.1 of the library 🙏

At least for now, I'm just updating it to rename the parcel output before packing the library.

Thank you for your patience on this!

@dominicbirch
Copy link
Owner

Oh! one other thing which is worth calling out here is that I also made the plugin emit an "updated" event when running in watch mode, so that if you want to log/node-notifier the out-of-process updates to .d.ts completing, you can use this to do so; was finding it difficult to get feedback back to webpack when running outside of the compilation so this was the best I could do for now 😅

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

Successfully merging a pull request may close this issue.

2 participants