ai / size-limit

Calculate the real cost to run your JS app or lib to keep good performance. Show error in pull request if the cost exceeds the limit.
MIT License
6.52k stars 1.82k forks source link

feat: add esbuild-why plugin #304

Closed unional closed 1 year ago

unional commented 1 year ago

fixes #302

unional commented 1 year ago

Seems like the check.esbuildMetafile I added in esbuild didn't reach esbuild-why for some reason.

ai commented 1 year ago

I am on vacation and will review it a little later

unional commented 1 year ago

Sure, enjoy your vacation! 🍹

unional commented 1 year ago

Welcome back! I'll need to do some update to this PR, like updating the dependency as now the function is properly exposed, and other things.

btw, is the step[81] looks ok? I saw the code accepts 1-99 steps and there are some conventions, but I don't know where should this fit into (step 61 or 81 is my guess).

ai commented 1 year ago

btw, is the step[81] looks ok?

There is no system. And since there is not a lot of plugins, just put it between necessary steps.

ai commented 1 year ago

Can we improve test coverage to fix CI?

unional commented 1 year ago

Can we improve test coverage to fix CI?

Sure, but right now I'm trying to see how to use it locally. i.e. how to set the --why and --saveBundle so that I can see it actually working.

I'll try to do a pnpm link to one of my repos locally to test it out.

In the doc, it mention about the config for webpack but not the config for esbuild. And maybe that's also something to improve upon.

ai commented 1 year ago

Sure, but right now I'm trying to see how to use it locally.

  1. Install dependencies pnpm install
  2. Create a test project in fixtures/ with all plugins in dependencies
  3. Call local size-limit script by ../package/size-limit/bin.js
unional commented 1 year ago

Install dependencies pnpm install Create a test project in fixtures/ with all plugins in dependencies Call local size-limit script by ../package/size-limit/bin.js

thx. I tried that but don't know if I'm doing it correctly.

I have added this in /fixtures/esbuild-why/package.json:

{
  "name": "esbuild-why",
  "private": true,
  "devDependencies": {
    "@size-limit/esbuild-why": ">= 0.0.0",
    "@size-limit/file": ">= 0.0.0",
    "size-limit": ">= 0.0.0"
  },
  "size-limit": [
    {
      "path": "index.js"
    }
  ]
}

with an empty index.js.

Call local size-limit script by ../package/size-limit/bin.js

I can't find where should I do this. From the path, seems like you are referring to calling size-limit from /packages/esbuild-why?

I'll find some time to work on this PR this week.

btw, I always get this timeout error when running the tests. I'm using WSL in Windows:

packages/time/test/get-running-time.test.js (15.731 s)
  â—Ź calculates running time

    thrown: "Exceeded timeout of 15000 ms for a test.
    Use jest.setTimeout(newTimeout) to increase the timeout value, if this is a long-running test."

      15 | })
      16 |
    > 17 | it('calculates running time', async () => {
         | ^
      18 |   let runTime = await getRunningTime(EXAMPLE)
      19 |   expect(runTime).toBeGreaterThan(0.01)
      20 |   expect(runTime).toBeLessThan(0.5)

      at Object.it (packages/time/test/get-running-time.test.js:17:1)
ai commented 1 year ago

I can't find where should I do this.

You should be in the fixture folder and call:

$ cd
size-limit/fixtures/integration
$ ../../packages/size-limit/bin.js
âś” Adding to empty webpack project
âś” Running JS in headless Chrome

  Package size is 58 B less than limit
  Size limit:   200 B
  Size:         142 B with all dependencies, minified and gzipped
  Loading time: 10 ms on slow 3G
  Running time: 84 ms on Snapdragon 410
  Total time:   94 ms
ai commented 1 year ago

btw, I always get this timeout error when running the tests. I'm using WSL in Windows:

I don’t have a Windows to investigate the reason without your help. But let’s ignore this test until we will not finish this PR.

unional commented 1 year ago

I don’t have a Windows to investigate the reason without your help. But let’s ignore this test until we will not finish this PR.

Sure, I'm ignoring that. :)

unional commented 1 year ago

Hi @ai, sorry for the long wait.

I have updated the code a bit to better handle the error message.

I have tested it and it will create a report.html at the folder. To get the same behavior as webpack-why, we need to either save the file at a temp folder and open the file in the browser.

ai commented 1 year ago

Nice. I fixed CI. Will review PR tomorrow (need to run to a party)

ai commented 1 year ago

Thanks! Released in 8.2.

unional commented 1 year ago

🎉