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: support `platform: 'node'` for esbuild plugin #291

Closed JounQin closed 2 years ago

JounQin commented 2 years ago

Add dependencies as external automatically at the same time

ai commented 2 years ago

What do you mean?

JounQin commented 2 years ago
 eslint-import-resolver-typescript on  master [✘+] ❯ p size-limit
⠋ Adding to empty esbuild project✘ [ERROR] Could not resolve "node:fs"

    lib/index.js:1:15:
      1 │ import fs from 'node:fs';
        ╵                ~~~~~~~~~

  You can mark the path "node:fs" as external to exclude it from the bundle, which will remove this
  error.

✘ [ERROR] Could not resolve "node:path"

    lib/index.js:2:17:
      2 │ import path from 'node:path';
        ╵                  ~~~~~~~~~~~

  You can mark the path "node:path" as external to exclude it from the bundle, which will remove
  this error.

✘ [ERROR] Could not resolve "node:url"

    lib/index.js:3:30:
      3 │ import { fileURLToPath } from 'node:url';
        ╵                               ~~~~~~~~~~

  You can mark the path "node:url" as external to exclude it from the bundle, which will remove this
  error.

✖ Adding to empty esbuild project
 ERROR  Error: Build failed with 3 errors:
lib/index.js:1:15: ERROR: Could not resolve "node:fs"
lib/index.js:2:17: ERROR: Could not resolve "node:path"
lib/index.js:3:30: ERROR: Could not resolve "node:url"
    at failureErrorWithLog (/Users/JounQin/Workspaces/GitHub/eslint-import-resolver-typescript/node_modules/.pnpm/esbuild@0.14.49/node_modules/esbuild/lib/main.js:1605:15)
    at /Users/JounQin/Workspaces/GitHub/eslint-import-resolver-typescript/node_modules/.pnpm/esbuild@0.14.49/node_modules/esbuild/lib/main.js:1251:28
    at runOnEndCallbacks (/Users/JounQin/Workspaces/GitHub/eslint-import-resolver-typescript/node_modules/.pnpm/esbuild@0.14.49/node_modules/esbuild/lib/main.js:1164:65)
    at buildResponseToResult (/Users/JounQin/Workspaces/GitHub/eslint-import-resolver-typescript/node_modules/.pnpm/esbuild@0.14.49/node_modules/esbuild/lib/main.js:1249:7)
    at /Users/JounQin/Workspaces/GitHub/eslint-import-resolver-typescript/node_modules/.pnpm/esbuild@0.14.49/node_modules/esbuild/lib/main.js:1358:14
    at /Users/JounQin/Workspaces/GitHub/eslint-import-resolver-typescript/node_modules/.pnpm/esbuild@0.14.49/node_modules/esbuild/lib/main.js:666:9
    at handleIncomingPacket (/Users/JounQin/Workspaces/GitHub/eslint-import-resolver-typescript/node_modules/.pnpm/esbuild@0.14.49/node_modules/esbuild/lib/main.js:763:9)
    at Socket.readFromStdout (/Users/JounQin/Workspaces/GitHub/eslint-import-resolver-typescript/node_modules/.pnpm/esbuild@0.14.49/node_modules/esbuild/lib/main.js:632:7)
    at Socket.emit (node:events:527:28)
    at addChunk (node:internal/streams/readable:315:12)
ai commented 2 years ago

You do not need Size Limi for Node’s project. Node’s project is not care about bundle size. It is only important for client-side or universal project.

ai commented 2 years ago

If you have an universal project, you need to have browsers section in your package.json https://github.com/ai/nanoid/blob/main/package.json#L36-L40

JounQin commented 2 years ago

It is not universal, it is node only.

JounQin commented 2 years ago
diff --git a/get-config.js b/get-config.js
index caf78653fda1c3464ffb12ff10e19cd82fce644a..ddd3f4a229791a633668bd66d7fb3a63217b5e70 100644
--- a/get-config.js
+++ b/get-config.js
@@ -16,7 +16,9 @@ module.exports = async function getConfig(limitConfig, check, output) {
     bundle: true,
     minifyWhitespace: true,
     minifyIdentifiers: true,
-    minifySyntax: true
+    minifySyntax: true,
+    
+    platform: 'node'
   }

   return config

Something like this.

ai commented 2 years ago

It is not universal, it is node only.

Why do you need Size Limit if it is Node-only project?

JounQin commented 2 years ago

Why do you need Size Limit if it is Node-only project?

Why not? Doesn't package size on node matter?

ai commented 2 years ago

Size Limit doesn’t track package size, it tracks bundle JS size.

It has a reason only for browser or universal projects.

There is no reason to use Size Limit for node-specific project.

To track package size, you can call npm publish and it will print the package size.

JounQin commented 2 years ago

There is no reason to use Size Limit for node-specific project.

Or third-party plugins like size-limit-* can be supported in https://github.com/ai/size-limit/blob/main/packages/size-limit/load-plugins.js?

ai commented 2 years ago

Yes, you can write custom plugin if you have a free time

JounQin commented 2 years ago

Yes, you can write custom plugin if you have a free time

But they are not supported for now, shall I add size-limit-* plugins support in a PR?

https://github.com/ai/size-limit/blob/dfd4b2757c0a7bf976f2532f144260029ca33772/packages/size-limit/load-plugins.js#L12

https://github.com/ai/size-limit/blob/dfd4b2757c0a7bf976f2532f144260029ca33772/packages/size-limit/load-plugins.js#L22

ai commented 2 years ago

We can but only if we will have a good reason

JounQin commented 2 years ago

Yes, you can write custom plugin if you have a free time

We can but only if we will have a good reason

Not all approaches meet your opinion, so third-party plugins can be chosen by users.

I want to use https://github.com/andresz1/size-limit-action to report on PR for node projects for whatever reason.

ai commented 2 years ago

OK, send PR with tests, docs end everything, so I will be able release it without my extra work

JounQin commented 2 years ago

OK, send PR with tests, docs end everything, so I will be able release it without my extra work

OK, will do it tomorrow. Thanks for accepting.

miridius commented 5 months ago

There is no reason to use Size Limit for node-specific project.

Sure there is! People complain constantly about the size of node_modules, it's become a meme that it is the heaviest thing in the universe. In general it's preferable to everyone to have their dependencies be smaller.

Don't forget that nodejs apps still get bundled in some way (e.g. docker image) and deployed, so file size matters there too

The main use case where it really matters though are serverless functions like AWS lambda, where having a large node app makes the startup much slower and every extra kB matters a lot.

ai commented 5 months ago

People complain constantly about the size of node_modules

Using esbuild will get you numbers irrelevant to npm package size. It change the files a lot.

The better way is to get all files size directly. For instance, you can use @size-limit/file plugin.

miridius commented 5 months ago

Ok but for serverless functions I would use esbuild to bundle my nodejs app into a single js file that can be used by the function, so that's the file size that matters

miridius commented 5 months ago

There's a solution for this now though which is:

ai commented 5 months ago

Ok but for serverless functions I would use esbuild to bundle my nodejs app into a single js file that can be used by the function, so that's the file size that matters

In this case you should run @size-limit/file after build step.

If you have your own esbuild with your own settings, you should not use other esbuild to estimate size (because it could have different settings).

Just call "test": "npm build && size-limit"