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

Update to webpack v5 #246

Merged
merged 2 commits into from Oct 7, 2021
Merged

Update to webpack v5 #246

merged 2 commits into from Oct 7, 2021

Conversation

ludofischer
Copy link
Contributor

Since more projects like Next.js are moving to webpack 5 by default, I've had another go at #205. Moving to webpack 5 also helps with #232 since webpack 5 itself has fewer dependencies.

I've used the previous PR #206 as a basis.
I've struggled with updating the expected bundle sizes in tests as I could not understand exactly how the constants inn

const WEBPACK_EMPTY_PROJECT = 962
were computed.
With webpack 5, the outputs seem to be sometimes larger and sometimes smaller.

Also I am not sure how to check that the bundle analyzer works correctly.
The previous blocker was webpack-bundle-analyzer not working with webpack 5 but it seems the most problematic issue has been solved in the mean time webpack-contrib/webpack-bundle-analyzer#447

Fixes #205

@ludofischer
Copy link
Contributor Author

More info on issues with webpack-bundle-analyzer: webpack-contrib/webpack-bundle-analyzer#466 Apparently since webpack now aggressively concatenates modules by default, sometimes it is just impossible to show how much each module contributes to the output size, since the module boundaries are lost in the output.

@ai
Copy link
Owner

ai commented Sep 22, 2021

I've struggled with updating the expected bundle sizes in tests as I could not understand exactly how the constants inn

You need to fix these constants according to the new webpack loader in the bundle (code which webpack adds in additional to user’s code).

We are using them to subtract webpack’s own code from the bundle to calculate the cost of added library:

libraryCost = projectWithLib - emptyProject

Change the value to have 0 in empty project test https://github.com/ai/size-limit/blob/main/packages/size-limit/test/run.test.js#L300-L306

And change IMPORT constants to have 1 on importing the single digit https://github.com/ai/size-limit/blob/main/packages/webpack/test/index.test.js#L240-L258

With webpack 5, the outputs seem to be sometimes larger and sometimes smaller.

It is OK. We are using gzip for many cases and some little juggling in size is expected (it is compromise between different ways to find the “cost”).

Also I am not sure how to check that the bundle analyzer works correctly.

If --why is showing Webpack Bundle Analyzer data it is OK. It is now Webpack Bundle Analyzer duty to find the best way to use webpack 5 API.

Anyway Webpack Bundle Analyzer data was never 100% accurate, it just help understand what happens, not find the exact numbers.

@ludofischer ludofischer force-pushed the webpack-5 branch 2 times, most recently from 30f2cb4 to 1f87c2a Compare October 2, 2021 18:23
@ludofischer
Copy link
Contributor Author

OK I think I have updated the constants in packages/webpack/index.js to have the correct values, so the empty project has 0 bytes and the single import has 1B. There are still some discrepancies with the old numbers in other tests, I think that because webpack does not include any runtime at all for basic examples, the gzip size ends up larger than the non-gzip size.

'integration-esm',
'index.js'
)]: '{ VERY_LONG_NAME_FOR_CONST_TO_TEST_TREE_SHAKING }'
[fixture('integration-esm', 'index.js')]:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have touched this file once and the git hook keeps wanting to reformat it although there are no changes.

@ai
Copy link
Owner

ai commented Oct 2, 2021

Looks great. Give me a few days, I need to release one fix in patch release of Size Limit, and then I will start preparing major release.

@ai ai merged commit dd75075 into ai:main Oct 7, 2021
@ai
Copy link
Owner

ai commented Oct 7, 2021

I will release it after this PR will be merged #217

@ludofischer ludofischer deleted the webpack-5 branch October 7, 2021 23:43
@ai
Copy link
Owner

ai commented Oct 9, 2021

Thanks for waiting. Released in 6.0.

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.

Update to webpack v5
2 participants