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

Esbuild plugin #270

Merged
merged 15 commits into from Nov 17, 2021
Merged

Esbuild plugin #270

merged 15 commits into from Nov 17, 2021

Conversation

Toomean
Copy link
Contributor

@Toomean Toomean commented Nov 14, 2021

Hello there! I've worked on a plugin for esbuild( according to #267 ).
Not sure about the needed requirements and the final point for this

Take a look, please

packages/esbuild/package.json Outdated Show resolved Hide resolved
@ai
Copy link
Owner

ai commented Nov 15, 2021

Looks excellent. We just need also check.import support to test libraries with tree-shaking.

Search for check.import and import in the project and you will find a code in webpack plugin (and config validation in size-limit) which you will need to back port (copy-pasting is OK, but if it will be possible to move some helpers to size-limit and then re-use it, it will better).

packages/esbuild/README.md Outdated Show resolved Hide resolved
packages/esbuild/package.json Outdated Show resolved Hide resolved
let runEsbuild = require('./run-esbuild')
let getConfig = require('./get-config')

const ESBUILD_EMPTY_PROJECT = 0
Copy link
Owner

@ai ai Nov 15, 2021

Choose a reason for hiding this comment

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

We will need tests with empty project to validate these constants.

The project with empty file should have 0 bytes.

The project with one digit export (and import config) should have 1 byte size.

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'm struggling with this a bit. Could you help me with that? Maybe we have an example of how to validate such kinds of constants

Copy link
Owner

Choose a reason for hiding this comment

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

Here is a test example for *_EMPTY_PROJECT_IMPORT and *_EMPTY_PROJECT_IMPORT_GZIP
https://github.com/ai/size-limit/blob/main/packages/webpack/test/index.test.js#L241-L268

Here is a test example for *_EMPTY_PROJECT and *_EMPTY_PROJECT_GZIP (yeap, it is in size-limit and not in webpack plugin for historical reason, feel free to move).
https://github.com/ai/size-limit/blob/main/packages/size-limit/test/run.test.js#L322-L328

packages/webpack/get-config.js Outdated Show resolved Hide resolved
@ai
Copy link
Owner

ai commented Nov 16, 2021

We still need to fix coverage and fix some strange Node.js 12 issue

@ai
Copy link
Owner

ai commented Nov 17, 2021

Test coverage shows that we need to calibrate (test) ESBUILD_EMPTY_PROJECT_IMPORT and ESBUILD_EMPTY_PROJECT_IMPORT_GZIP.

Also, we still have Node.js 12 problem:

> error: Could not resolve "/home/runner/work/size-limit/size-limit/packages/esbuild/test/fixtures/unknown.js"

 > error: Refusing to overwrite input file "../../../../../tmp/size-limit-qzG5j_Arw23-2HlwpuSXJ/index.js" (use "allowOverwrite: true" to allow this)

 > error: Refusing to overwrite input file "../../../../../tmp/size-limit-8Rz_8m1YKc_k-mX_IJzsY/index.js" (use "allowOverwrite: true" to allow this)

@Toomean
Copy link
Contributor Author

Toomean commented Nov 17, 2021

Can't reproduce errors from actions on my local machine, will investigate it on another os. As I see, this problem appears on every node version

@ai
Copy link
Owner

ai commented Nov 17, 2021

Can't reproduce errors from actions on my local machine

It could be related to CI environment also

@ai
Copy link
Owner

ai commented Nov 17, 2021

I pushed merge commit to run CI

@ai ai merged commit 5287e3d into ai:main Nov 17, 2021
@ai
Copy link
Owner

ai commented Nov 17, 2021

Greta work. Thanks.

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.

None yet

2 participants