Core: Async load presets, replace interpret with esbuild-register

[ndelangen]

@tmeasday this kinda replaces your PR: #18025

It does not preserve the import(), but that's because there's no way to make that work with TypeScript AFAICS.

This adds an esbuild-register as soon as it's needed, but only once; this mutates the require method in Node to run code through esbuild (which does support TypeScript and other stuff.

I need to experiment more, but this also seems much faster!

@shilman the removes the interpreter "nonsense", too..

[nx-cloud]

☁️ Nx Cloud Report

CI is running/has finished running commands for commit 645e1b8. As they complete they will appear below. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this branch

---

✅ Successfully ran 1 target

• nx run-many --target=prepare --all --parallel=8 --max-parallel=15 -- --reset --optimized

---

Sent with 💌 from NxCloud.

[tmeasday]

@ndelangen so does this mean at the point we first require a user file we register esbuild and then from then on everything that gets required (including our own code) gets passed through it?

I guess in node, everything of our own will have already been required before this point (no code splitting or async imports). I guess I am just a tiny bit concerned about inconsistencies of in some circumstances importing our code through esbuild and in others not. What's your thinking here @ndelangen?

[ndelangen]

Every time a user's file get loaded I register the esbuild-register, then when it's loaded I unload it.

[tmeasday]

then when it's loaded I unload it.

How are you doing that? I'm not seeing it..?

[ndelangen]

Hmm, I must have undone that work for some reason @tmeasday I'll change it to what I had.

[tmeasday]

LGTM!

[IanVS]

I'd love to test this out in my vite project, but as far as I know, the future branch is not compatible with the vite-builder, right?

[ndelangen]

@tmeasday I think there's race conditions when registering and unregistering the esbuild loader in storyshots.

😢

[ndelangen]

the interpret package didn't unregister either btw

[ndelangen]

@tmeasday I agree with you that registering and then unregistering is better, and safer. But somehow storyshots is dependent on the registering happening globally and staying registered.
This is also what happened with the interpret package.

I do not want to invest any time into storyshots, mainly because I'd rather work on improving testing library to replace it.
But also I'm getting absolutely NO, ZERO indication where the problem is from storyshots. Pinning it down is going to eat away my time and energy.

I propose we merge as is.

[tmeasday]

@ndelangen I would be happy to spend some time investigating, I've seen errors like this from SS before and I generally have figured them out.

However, I did:

git checkout 18064b2
yarn bootstrap --core
yarn test

and all the tests passed 🤷

I'll rerun the job in CI.

[ndelangen]

I also could not reproduce locally, at first, then after some trial and error I could get some story shots test to fail, but when running them in isolation they'd succeed.

[tmeasday]

As discussed I think the register-once approach is probably better anyway. I would consider registering right at the start, rather than when we load our first user file; but if that's what we were doing before it's probably fine.

[shilman]

Super excited about this @ndelangen 🚀