Skip to content

Conversation

@MarcelRobitaille
Copy link
Collaborator

The bundle analyzer plugin is very slow. When I am running with npm run dev, this plugin takes a few seconds after each compile. This really slows down the code-test loop for no reason (I think 99% of the time, we are fixing bugs and adding new features, not optimizing the bundle). Therefore, this PR disables this plugin by default, and adds another npm script to build with this plugin enabled.

I also fixed npm run build-dev using webpack.config.js instead of webpack.devel.js, which I think was a mistake.

In summary:

  • npm run build is unchanged
  • npm run build-dev now uses webpack.devel.js and the bundle analyzer plugin is disabled
  • npm run build-bundle-analyzer is a new script that uses webpack.devel.js and the bundle analyzer plugin is enabled
  • npm run dev has the bundle analyzer plugin is disabled

I think this was just a mistake in the configuration. Please correct me
if this was not the intention.

Signed-off-by: Marcel Robitaille <[email protected]>
The bundle analyzer plugin is very slow. When I am running with `npm run
dev`, this plugin takes a few seconds after each compile. This really
slows down the code-test loop for no reason. Therefore, this PR disables
this plugin by default, and adds another npm script to build with this
plugin enabled.

Signed-off-by: Marcel Robitaille <[email protected]>
Signed-off-by: Marcel Robitaille <[email protected]>
@MarcelRobitaille
Copy link
Collaborator Author

Now that this is its own command, we should consider setting openAnalyzer to true. We could also put this behind a flag so you can configure the setting easily from the command line without having to change the configuration file. Something like npm run build-bundle-analyzer --open-analyzer.

@github-actions
Copy link

Unit Test Results

     27 files       27 suites   6m 33s ⏱️
   476 tests    476 ✔️ 0 💤 0
4 284 runs  4 283 ✔️ 1 💤 0

Results for commit 9c3416f.

@christianlupus
Copy link
Collaborator

Now that this is its own command, we should consider setting openAnalyzer to true. We could also put this behind a flag so you can configure the setting easily from the command line without having to change the configuration file. Something like npm run build-bundle-analyzer --open-analyzer.

This seems good. Do you want to implement this as well? Then, we can wait with fixing and merging the changes later.

@christianlupus christianlupus added this to the Release 0.9.16 milestone Oct 19, 2022
@MarcelRobitaille
Copy link
Collaborator Author

Do you want to implement this as well?

Sorry, I'm not sure what "this" refers to. Do you prefer:

  1. Open by default
  2. Add a flag like npm run build-bundle-analyzer --open-analyzer

@christianlupus
Copy link
Collaborator

Ahh, sorry. I meant to implement the CLI option like --no-open-analyzeror similar. Otherwise one might be better off to use the npm target serve if the analyzer is not needed. Or am I missing a use case?

@MarcelRobitaille
Copy link
Collaborator Author

MarcelRobitaille commented Oct 20, 2022

Otherwise one might be better off to use the npm target serve if the analyzer is not needed. Or am I missing a use case?

That's exactly the use case, but the change is that if the bundle analyzer is not needed, it's not even executed as it's very slow.

The command npm run build-bundle-analyzer is intended only to be used when the bundle analyzer is needed. It's this command that could either open the result all the time, or have a flag to open it or not. I think the options are:

  1. Never open the result in a browser (current situation)
  2. Always open the result in a browser
  3. Open the result only if npm run build-bundle-analyzer --open-analyzer
  4. Open the result unless npm run build-bundle-analyzer --no-open-analyzer

Again, these options are only for npm run build-bundle-analyzer. Other npm scripts now have nothing to do with the bundle analyzer.

The decision depends on what is the most useful if you are working on optimizing the bundle size. In this case, you will probably be running the bundle analyzer multiple times to compare the results. In that case, maybe opening the result in a new tab every run is the best thing to do.

Copy link
Collaborator

@christianlupus christianlupus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@christianlupus christianlupus merged commit 69724a0 into nextcloud:master Oct 20, 2022
@MarcelRobitaille MarcelRobitaille deleted the speed-up-webpack branch October 20, 2022 20:59
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 this pull request may close these issues.

2 participants