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

feat(builder): build plugins with esbuild #2717

Merged
merged 7 commits into from
Jun 3, 2021
Merged

Conversation

paul-soporan
Copy link
Member

@paul-soporan paul-soporan commented Apr 8, 2021

What's the problem this PR addresses?

We currently use ESBuild to build the bundle and Webpack to build the plugins.

How did you fix it?

Plugins will now also be built using ESBuild. I kept the dynamic lib magic by creating an inline plugin. I also kept the wrapper by using banner and footer.

Stats for compiling @yarnpkg/plugin-workspace-tools:

Before:

  • Time: 17s 177ms
  • Bundle Size: 47.53 KB

After:

  • Time: 3s 911ms
  • Bundle Size: 48.73 KB (very small increase, part of it because of the new //prettier-ignore comment in the wrapper)

Note: I think building plugin-version might be a bit broken currently because of how @yarnpkg/esbuild-plugin-pnp handles optional peer deps, but I'll investigate tomorrow after getting some sleep.

Checklist

  • I have set the packages that need to be released for my changes to be effective.
  • I will check that all automated PR checks pass before the PR gets reviewed.

@paul-soporan paul-soporan requested a review from arcanis as a code owner May 8, 2021 23:37
@paul-soporan
Copy link
Member Author

I decided to make @yarnpkg/esbuild-plugin-pnp downgrade errors to warnings on dynamic resolve calls until evanw/esbuild#1127 is addresed so that this PR can be unblocked.

@paul-soporan
Copy link
Member Author

Waiting on #2863.

@paul-soporan paul-soporan requested a review from merceyz June 1, 2021 10:04
@merceyz merceyz changed the title build(builder): build plugins with esbuild feat(builder): build plugins with esbuild Jun 1, 2021
merceyz
merceyz previously approved these changes Jun 1, 2021
.yarn/versions/00a383da.yml Outdated Show resolved Hide resolved
@@ -0,0 +1,25 @@
releases:
"@yarnpkg/builder": major
"@yarnpkg/cli": major
Copy link
Member

Choose a reason for hiding this comment

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

I don't see why the cli needs a release 🤔

Copy link
Member Author

Choose a reason for hiding this comment

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

I bumped it because updating ESBuild might cause the generated bundle to be different 🤔

Copy link
Member

Choose a reason for hiding this comment

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

Oh yeah, due to esbuild not always including __esModule anymore it might break plugins that depends on it existing 🤔

@arcanis
Copy link
Member

arcanis commented Jun 3, 2021

LGTM - wdyt, @merceyz?

@arcanis arcanis requested a review from merceyz June 3, 2021 09:58
Copy link
Member

@merceyz merceyz left a comment

Choose a reason for hiding this comment

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

I'm a bit worried about #2717 (comment) but otherwise LGTM

@arcanis arcanis enabled auto-merge (squash) June 3, 2021 11:22
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.

3 participants