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

Size is reported as 0 bytes since upgrading to 6.x #261

Open
ezzatron opened this issue Oct 13, 2021 · 23 comments
Open

Size is reported as 0 bytes since upgrading to 6.x #261

ezzatron opened this issue Oct 13, 2021 · 23 comments

Comments

@ezzatron
Copy link

I'm trying to upgrade size-limit to 6.0.3 in my projects. I'm also using @size-limit/preset-small-lib at the same version. I haven't made any changes to my config or code, but I'm now seeing sizes of 0 bytes:

/opt/hostedtoolcache/node/14.18.0/x64/bin/npx size-limit --json
[
  {
    "name": "dist/index.js",
    "passed": true,
    "size": 0
  },
  {
    "name": "dist/index.es.js",
    "passed": true,
    "size": 0
  }
]

Versus the output I get from size-limit@^5.0.3:

/opt/hostedtoolcache/node/14.18.0/x64/bin/npx size-limit --json
[
  {
    "name": "dist/index.js",
    "passed": true,
    "size": 56
  },
  {
    "name": "dist/index.es.js",
    "passed": true,
    "size": 66
  }
]

This results in reports indicating that I've removed 100% of my code:

Screen Shot 2021-10-13 at 10 42 51 am

@ai
Copy link
Owner

ai commented Oct 13, 2021

It could be related to webpack 4→5 changes in 6.0.

  1. Why do you use Rollup in your project?
  2. How can I reproduce the bug?

@ezzatron
Copy link
Author

  1. I use Rollup to produce both CommonJS and ECMAScript module versions of my libraries from the original TypeScript source.
  2. The following steps should reproduce the problem locally:
git clone git@github.com:snout-router/regexp.git
cd regexp
git switch upgrade-size-limit
yarn install
node_modules/.bin/size-limit

The yarn install should trigger dist to be created (it happens in a prepare script in package.json).

@ai
Copy link
Owner

ai commented Oct 14, 2021

Change path: 'dist/index.js',path: './dist/index.js', (add ./ to the start of the path)

@ai
Copy link
Owner

ai commented Oct 14, 2021

I added a test for .-less path 75858fd but was not able to reproduce the bug.

It is DX issue, but since we found a fix and can’t easily reproduce it in test environment, I am closing the bug.

Bug I will be happy if you will reproduce the bug in test environment so we will be able to fix it in size-limit.

@ai ai closed this as completed Oct 14, 2021
@ezzatron
Copy link
Author

Unfortunately this doesn't fix the issue for me. I get the same result running locally:

$ node_modules/.bin/size-limit
✔ Adding to empty webpack project

  ./dist/index.js
  Package size is 100 B less than limit
  Size limit: 100 B
  Size:       0 B   with all dependencies, minified and gzipped

  ./dist/index.es.js
  Package size is 100 B less than limit
  Size limit: 100 B
  Size:       0 B   with all dependencies, minified and gzipped

And the same 0 byte result happens under GHA: https://github.com/snout-router/regexp/runs/3893923974?check_suite_focus=true#step:4:36

@ai ai reopened this Oct 14, 2021
@ezzatron
Copy link
Author

Sorry. I'll try to see if I can put together a simpler repro case.

@ai
Copy link
Owner

ai commented Oct 14, 2021

Oops. I forget to remove another trick.

It help me adding import: '{ escape }', to checks.

@ai
Copy link
Owner

ai commented Oct 14, 2021

I am working on a better solution

@ezzatron
Copy link
Author

It help me adding import: '{ escape }', to checks.

I get results by adding this 👍 Although if it's possible it'd be great to have an option that was equivalent to import *.

@ai
Copy link
Owner

ai commented Oct 14, 2021

@ludofischer webpack 5 started to remove any exports from bundles if we do not pass import option.

Any idea how to disable it for non-import cases?

@ezzatron
Copy link
Author

You might also be interested to know that the ./ path prefix was not necessary to get results, only manually specifying the import option was necessary: https://github.com/snout-router/regexp/runs/3894148976?check_suite_focus=true#step:5:25

P.S. I'm also getting some pretty surprising results for the size of the CJS version, but I'll make another issue for that if necessary.

@ai
Copy link
Owner

ai commented Oct 14, 2021

@ludofischer I added shakable-webpack-5 with a test for this case

@yumauri
Copy link

yumauri commented Oct 14, 2021

P.S. I'm also getting some pretty surprising results for the size of the CJS version, but I'll make another issue for that if necessary.

In my case all .cjs files' sizes were increased approximately by 140-150 bytes.
I think webpack starts adding some kind of wrappers for common js modules.

Is there any way to preserve temporary bundles' files, in order to check manually, by looking through them with my own eyes, whats going on?

@ai
Copy link
Owner

ai commented Oct 15, 2021

In my case all .cjs files' sizes were increased approximately by 140-150 bytes.

@yumauri we already subtract wrapper size from the bundle size.

In my case, I got 20-30 extra bytes, but because of the opposite reason. webpack 5 wrapper is very small and gzip now can use it to reduce size of your JS code.

So for my case the result become bigger, but a little more fair.

@ludofischer
Copy link
Contributor

ludofischer commented Oct 16, 2021

Any idea how to disable it for non-import cases?

Sorry, I don't know about this. I would need to look into the webpack docs :-) Apparently you can use https://webpack.js.org/configuration/optimization/#optimizationusedexports but it looks like it's going to disable tree shaking even for transitive dependencies, so it will report a larger bundle size than what will happen in production. With tree-shaking the question 'how much does this add to the bundle' has more than one answer. It might really be zero if the user just imports your library without calling it…

In my case all .cjs files' sizes were increased approximately by 140-150 bytes.

When we calculate the bundle size, we subtract the 'empty' bundle size to get the final bundle size. With webpack 4 even bundling empty code would create some wrappers. I think what might be happening is that with webpack 5 the empty bundle size is almost 0, so adding any code makes a larger difference.
Why not just report the webpack bundle size without performing any subtraction, though?

@ezzatron
Copy link
Author

Sorry, I don't know about this. I would need to look into the webpack docs :-) Apparently you can use https://webpack.js.org/configuration/optimization/#optimizationusedexports but it looks like it's going to disable tree shaking even for transitive dependencies, so it will report a larger bundle size than what will happen in production. With tree-shaking the question 'how much does this add to the bundle' has more than one answer. It might really be zero if the user just imports your library without calling it…

To be clear, what I expected by leaving off import was essentially that it would mean the same thing as import * as x from 'my-module'. Basically, what is the size if you use every named and default export the module has. I believe this was the behaviour under the previous version of size-limit.

Why not just report the webpack bundle size without performing any subtraction, though?

My guess is that most developers would want to know how much "extra" bundle size they'd be getting by adding the library to an existing project that already uses Webpack, and hence they've already decided that they're comfortable with the overhead of Webpack itself. It's much less useful to know how big a library would be if you bundled only that singular library and nothing else.

@ai
Copy link
Owner

ai commented Oct 18, 2021

@ludofischer we can add some optional like importAll: true or import: "*" and use import-option code to generate file like:

import * as all from '../entry.js'
console.log(all)

@ludofischer
Copy link
Contributor

@ludofischer we can add some optional like importAll: true or import: "*" and use [import-option code]

For my own libraries, I would prefer to use the path option to point to example code that uses the library instead of the library entry point. That way I can measure the size impact for any usage pattern I want. I feel like currently there's more of a documentation issue, it's not clear how the number that size-limit outputs relates to the size for real-world usage.

@ai
Copy link
Owner

ai commented Oct 18, 2021

For my own libraries, I would prefer to use the path option to point to example code that uses the library instead of the library entry point.

The problem of this way is that we will not substract this entry point from the library size.

It is OK for 1-10K projects, but not so good for projects like from issue owner.

@ludofischer
Copy link
Contributor

It is OK for 1-10K projects, but not so good for projects like from issue owner.

What about outputting best/worst case numbers by default (so both the current configuration and using the console.log(all) trick)? That way people do not need to bother with extra options.

@ai
Copy link
Owner

ai commented Oct 18, 2021

What about outputting best/worst case numbers by default (so both the current configuration and using the console.log(all) trick)? That way people do not need to bother with extra options.

Hm. I like the idea.

@ludofischer
Copy link
Contributor

Hm. I like the idea.

Which limit would make CI fail though? Should the limit in the size-limit config now mean 'worst case limit'?

@ai
Copy link
Owner

ai commented Oct 18, 2021

Which limit would make CI fail though? Should the limit in the size-limit config now mean 'worst case limit'?

My idea is to use import * as all from '${path}'; console.log(all) test on no import: "{ some }" option.

But we need to be sure that we can use import * for file without exports.

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

No branches or pull requests

4 participants