feat(webpack): introduce experimental esbuild transpilation #1632

jhiode · 2021-02-26T14:49:19Z

To test this just add esbuild and esbuild-loader as dependencies to hops-template-redux.

$ time -l yarn hops build -p
hops-template-react:info running 'build' in 'production' mode
hops-template-react:info bundling 'node' finished after 5.6s (2.93 MB)
- server.js (2.93 MB)
hops-template-react:info bundling 'build' finished after 12.9s (318 kB)
- hops-template-react-1-71d927b348e8.js (279 kB)
- hops-template-react-4531ec959528.js (38.2 kB)
- main-e94a689b8054.css (207 B)
✨  Done in 17.02s.

$ time -l yarn hops build -p --experimentalEsbuild
hops-template-react:info running 'build' in 'production' mode
hops-template-react:info bundling 'node' finished after 4.2s (2.59 MB)
- server.js (2.59 MB)
hops-template-react:info bundling 'build' finished after 4s (223 kB)
- hops-template-react-1-89661b912d7e.js (211 kB)
- hops-template-react-202879a799d9.js (11.7 kB)
- main-e94a689b8054.css (207 B)
✨  Done in 8.63s.

Memory consumption also goes down from ~340MB to ~240MB.

packages/webpack/mixins/esbuild/mixin.core.js

packages/webpack/lib/plugins/log.js

packages/webpack/mixins/esbuild/mixin.core.js

jhiode · 2021-03-04T18:30:24Z

Jest is also mostly working now. We have one Flow test that isn't working, which is fine I guess and there seems to be another import syntax that is hard to match via regex. Not sure what we do about that.

The syntax I am talking about:

hops/packages/jest-preset/helpers/__tests__/fixture/code-02.txt

Line 3 in 5ba1ffc

export const Foo = importComponent(() => import('./foo'), (ns) => ns.Foobar);

herschel666

Regarding the import component loader: couldn't we leverage acorn when we find ocurrences of importComponent( in the code and then basically replicating thte respective babel loader code?

Or we check whether there's a second parameter passed into importComponent, and if there is, issue an error telling the user, that Hops's esbuild-feature currently only supports default exports in importComponent. 🤷‍♂️

packages/webpack/mixins/esbuild/importComponentTransform.js

jhiode · 2021-03-05T09:38:15Z

Btw, in ESBuild there are four different loaders: js, jsx, ts, tsx. For webpack I'm just using jsx for jsx? and tsx for tsx?. For JavaScript that shouldn't be an issue as any JS file is also a valid JSX file. However, that is not true for TS and TSX.
So for the Jest loader I just kept the default to use the extension to detect which loader should be used. For that, I had to rename a lot of (internal) files though (see d75c5d7).

I think I should align that to the webpack loader as well, but that would require everyone who wants to use this esbuild feature to possibly fix their file extensions.

herschel666 · 2021-03-05T09:51:49Z

I think I should align that to the WebPack loader as well, but that would require everyone who wants to use this esbuild feature to possibly fix their file extensions.

I think it would be fine to introduce this requirement for an experimental feature coming with a major update. But if we want to go further down the esbuild road, we'll eventually have to introduce that requirement to everybody. I personally think that's appropriate. Do we all agree on that?

ZauberNerd · 2021-03-08T15:54:25Z

Regarding the import component loader: couldn't we leverage acorn when we find ocurrences of importComponent( in the code and then basically replicating thte respective babel loader code?

I took a stab at that a few days ago, unfortunately, the webpack parser APIs are not so easy and also pretty much not documented.
I suppose it should look something like that: f81cb4a, but I'm not sure 😆
There's only 11 results on GitHub for parser.hooks.call.for: https://grep.app/search?q=parser.hooks.call.for :(

I guess it'd be easier to continue with the RegExp based solution if we want to finish it in time for Hops 14.

Or we check whether there's a second parameter passed into importComponent, and if there is, issue an error telling the user, that Hops's esbuild-feature currently only supports default exports in importComponent. 🤷‍♂️

I'd prefer, if we can handle named imports, I'll take a look at the regex, so see if I can figure out a way.

ZauberNerd · 2021-03-12T09:19:41Z

Another thing to keep in mind: There seems to be some incompatibility when using esbuild artifacts in webpack: evanw/esbuild#758 (see also PR 2855 in crate). We should evaluate if that is also the case here.

ZauberNerd · 2021-03-15T16:02:57Z

packages/jest-preset/transforms/esbuild.js

+
+module.exports = {
+  process(content, filename, config, opts) {
+    content = content.replace(


Another idea: We could do a regex test for importComponent and if we find it, we then load acorn / babel / whatever to do the actual AST transform. That would be safer, because a regex replace alone can't catch things like import { importComponent as importC } from 'hops' or it might replace the wrong thing (if for some reason a file contains a function call to importComponent() from a different package.

ZauberNerd

Documentation is missing. For testing you need to set USE_EXPERIMENTAL_ESBUILD and otherwise you use --experimental-esbuild or the env var. It would be good to have that documented ;)

packages/react/build/mixin.core.js

ZauberNerd · 2021-03-24T10:24:49Z

packages/styled-components/mixin.core.js

+
+    if (experimentalEsbuild) {
+      console.warn(
+        'The experimental esbuild transpilation does not support styled components yet!'


It doesn't support it for SSR, right? But it would still give useful output in dev mode?
Maybe change this warning a bit, to indicate that it can still be used in dev mode.

I haven't really looked into it so I'm not sure yet what works and what doesn't.

packages/webpack/mixins/esbuild/mixin.core.js

herschel666 · 2021-03-24T10:50:34Z

packages/typescript/mixin.core.js

+      const {
+        include,
+        exclude,
+        use: [{ loader, options }, ...loaders],


Suggested change

use: [{ loader, options }, ...loaders],

use: [{ loader: esbuildLoader, options }, ...loaders],

To make it a little more clear what's going on here.

packages/spec/integration/esbuild-typescript/yarn.lock

ZauberNerd · 2021-03-24T13:52:28Z

packages/spec/integration/esbuild-typescript/package.json

@@ -0,0 +1,32 @@
+{


Should we also run a jest test in this integration test, to demonstrate how it works?

systemboogie

Even if that command-line flag has an "experimental" in its name I wonder: Shouldn't Hops print a warning/info saying that

it is experimental
some things may not work as expected (apart from styled components)
and that a production build is created differently

as we do not support styled components yet

jhiode · 2021-03-24T15:58:11Z

bors merge

bors · 2021-03-24T16:07:46Z

Build succeeded:

canarist/internal-hops
canarist/internal-workshop
tests (12.x)
tests (14.x)
tests (15.x)

In case the `--experimental-esbuild` mode is used, we have a small webpack loader to handle the `importComponent` transformation. This had been introduced in #1632 to be a quick & dirty regex based replacer. With this commit we now have a webpack loader that has a fast-exit path in case `importComponent` is not used. Otherwise it will transpile the source code using the proper babel plugin. Co-authored-by: Markus Wolf <markus.wolf@new-work.se> Co-authored-by: Philipp Hinrichsen <philipp.hinrichsen@new-work.se> Co-authored-by: Robert Kowalski <robert.kowalski@new-work.se>

2070: Fix `importComponent` r=ZauberNerd a=ZauberNerd Ref: #1632, #1976 Todo: - [x] What happens if we import the same file from different files? I suspect our babel plugin will generate different chunk names which should in fact not be different. So maybe our hash should not include the filepath of the importing component but only the filepath of the imported component. - seems to create separate entries in the `namedChunkGroups` which point to the same asset. <details> <summary>Bors merge bot cheat sheet</summary> We are using [bors-ng](https://github.com/bors-ng/bors-ng) to automate merging of our pull requests. The following table provides a summary of commands that are available to reviewers (members of this repository with push access) and delegates (in case of `bors delegate+` or `bors delegate=[list]`). | Syntax | Description | | --- | --- | | bors merge | Run the test suite and push to master if it passes. Short for "reviewed: looks good." | | bors merge- | Cancel an r+, r=, merge, or merge= | | bors try | Run the test suite without pushing to master. | | bors try- | Cancel a try | | bors delegate+ | Allow the pull request author to merge their changes. | | bors delegate=[list] | Allow the listed users to r+ this pull request's changes. | | bors retry | Run the previous command a second time. | This is a short collection of opinionated commands. For a full list of the commands read the [bors reference](https://bors.tech/documentation/). </details> Co-authored-by: Björn Brauer <bjoern.brauer@new-work.se> Co-authored-by: Robert Kowalski <robert.kowalski@new-work.se>

In case the `--experimental-esbuild` mode is used, we have a small webpack loader to handle the `importComponent` transformation. This had been introduced in #1632 to be a quick & dirty regex based replacer. With this commit we now have a webpack loader that has a fast-exit path in case `importComponent` is not used. Otherwise it will transpile the source code using the proper babel plugin. Co-authored-by: Markus Wolf <markus.wolf@new-work.se> Co-authored-by: Philipp Hinrichsen <philipp.hinrichsen@new-work.se> Co-authored-by: Robert Kowalski <robert.kowalski@new-work.se>

jhiode added 🔨 build ⏱ performance 📦 master Apply this label to a pull request, if it has to be cherry-picked to the maste-branch. labels Feb 26, 2021

jhiode changed the title ~~@jhiode feat(webpack): introduce experimental esbuild transpilation~~ feat(webpack): introduce experimental esbuild transpilation Feb 26, 2021

herschel666 reviewed Feb 26, 2021

View reviewed changes

packages/webpack/mixins/esbuild/mixin.core.js Outdated Show resolved Hide resolved

packages/webpack/lib/plugins/log.js Outdated Show resolved Hide resolved

jhiode force-pushed the esbuild branch 2 times, most recently from 9ed63c5 to 3bb64c4 Compare March 2, 2021 12:41

herschel666 reviewed Mar 2, 2021

View reviewed changes

packages/webpack/mixins/esbuild/mixin.core.js Outdated Show resolved Hide resolved

jhiode force-pushed the esbuild branch 2 times, most recently from 0d7efac to d75c5d7 Compare March 4, 2021 18:24

herschel666 reviewed Mar 5, 2021

View reviewed changes

packages/webpack/mixins/esbuild/importComponentTransform.js Outdated Show resolved Hide resolved

jhiode force-pushed the esbuild branch from 0d8c6e7 to e2b1f9b Compare March 5, 2021 17:31

ZauberNerd reviewed Mar 15, 2021

View reviewed changes

jhiode force-pushed the esbuild branch 2 times, most recently from a3ffe94 to d2cad6d Compare March 23, 2021 14:57

fix(webpack): ignore source-maps in total asset size

dc59abd

jhiode force-pushed the esbuild branch from d2cad6d to a48a7bd Compare March 23, 2021 15:09

ZauberNerd reviewed Mar 24, 2021

View reviewed changes

herschel666 suggested changes Mar 24, 2021

View reviewed changes

ZauberNerd reviewed Mar 24, 2021

View reviewed changes

packages/spec/integration/esbuild-typescript/yarn.lock Outdated Show resolved Hide resolved

ZauberNerd reviewed Mar 24, 2021

View reviewed changes

systemboogie reviewed Mar 24, 2021

View reviewed changes

jhiode added 3 commits March 24, 2021 16:22

feat(webpack): introduce experimental esbuild transpilation

1746b4d

feat(typescript): add support for esbuild

13028d2

feat(react): add support for esbuild

6a416a8

jhiode added 4 commits March 24, 2021 16:22

chore(styled-components): warn when esbuild is used

5220018

as we do not support styled components yet

feat(jest-preset): introduce experimental esbuild transpilation

6a2845e

chore: rename files which use JSX syntax to have the .jsx extension

56ad96e

chore: add esbuild example project

f8f1ad3

jhiode force-pushed the esbuild branch from e9f69d2 to f8f1ad3 Compare March 24, 2021 15:22

jhiode marked this pull request as ready for review March 24, 2021 15:23

ZauberNerd approved these changes Mar 24, 2021

View reviewed changes

herschel666 approved these changes Mar 24, 2021

View reviewed changes

bors bot merged commit 051da1c into master Mar 24, 2021

bors bot deleted the esbuild branch March 24, 2021 16:07

ZauberNerd mentioned this pull request Jan 31, 2022

Fix importComponent #2070

Merged

1 task

hops-release-bot bot mentioned this pull request Feb 16, 2022

Fix importComponent (v15.x) #2078

Merged

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(webpack): introduce experimental esbuild transpilation #1632

feat(webpack): introduce experimental esbuild transpilation #1632

jhiode commented Feb 26, 2021

jhiode commented Mar 4, 2021 •

edited

herschel666 left a comment

jhiode commented Mar 5, 2021 •

edited

herschel666 commented Mar 5, 2021

ZauberNerd commented Mar 8, 2021

ZauberNerd commented Mar 12, 2021

ZauberNerd Mar 15, 2021

ZauberNerd left a comment

ZauberNerd Mar 24, 2021

jhiode Mar 24, 2021

herschel666 Mar 24, 2021

ZauberNerd Mar 24, 2021

systemboogie left a comment •

edited

jhiode commented Mar 24, 2021

bors bot commented Mar 24, 2021

	use: [{ loader, options }, ...loaders],
	use: [{ loader: esbuildLoader, options }, ...loaders],

feat(webpack): introduce experimental esbuild transpilation #1632

feat(webpack): introduce experimental esbuild transpilation #1632

Conversation

jhiode commented Feb 26, 2021

jhiode commented Mar 4, 2021 • edited

herschel666 left a comment

Choose a reason for hiding this comment

jhiode commented Mar 5, 2021 • edited

herschel666 commented Mar 5, 2021

ZauberNerd commented Mar 8, 2021

ZauberNerd commented Mar 12, 2021

ZauberNerd Mar 15, 2021

Choose a reason for hiding this comment

ZauberNerd left a comment

Choose a reason for hiding this comment

ZauberNerd Mar 24, 2021

Choose a reason for hiding this comment

jhiode Mar 24, 2021

Choose a reason for hiding this comment

herschel666 Mar 24, 2021

Choose a reason for hiding this comment

ZauberNerd Mar 24, 2021

Choose a reason for hiding this comment

systemboogie left a comment • edited

Choose a reason for hiding this comment

jhiode commented Mar 24, 2021

bors bot commented Mar 24, 2021

jhiode commented Mar 4, 2021 •

edited

jhiode commented Mar 5, 2021 •

edited

systemboogie left a comment •

edited