PepsRyuu / nollup

Rollup compatible development bundler for fast rebuilds and HMR.
MIT License
488 stars 28 forks source link

Live bindings - enum export issue #116

Closed high1 closed 3 years ago

high1 commented 4 years ago

I have an issue for which I don't have a solution, and it works properly when building for production. The issue is simple, the code is transpiled with Babel from Typescript using solid preset - basically, when running on nollup, the code breaks not being able to found an enum entry - simple enum, not const enum. I would appreciate your input, if and when you have the time. solid-rollup-ts

PepsRyuu commented 4 years ago

Looks like the enum is being transpiled in an incompatible way.

var MyEnum;; __e__('MyEnum', MyEnum);

(function (MyEnum) {
  MyEnum[MyEnum["first"] = 30] = "first";
})(MyEnum || (MyEnum = {}));

It's being transpiled in a way that the code is relying on live-bindings, which Nollup doesn't support. Live bindings have been intentionally avoided because they're costly in build performance and they break debugging symbols, and are usually an edge case that's often not needed. It works fine with Rollup as Rollup avoids the live-binding problem by scope hoisting everything into a single function.

It might be possible to adjust Nollup so that exports are always the last thing to happen in any file, but I'm not sure what consequences this might have. I'll test this and see how it goes.

As a workaround, you can split the enum declaration from the export declaration, and it will work just fine:

enum MyEnum {
    first = 30
}

export { MyEnum }

Hope this helps! :)

high1 commented 4 years ago

Yes it does! Would you like me to close this, or wait for the export test results?

PepsRyuu commented 4 years ago

Going to try and see if this can be resolved internally in Nollup before closing. :)

PepsRyuu commented 4 years ago

Researching into this more, while it's possible to resolve this specific issue above by deferring exports to the end of the file, it unfortunately breaks HMR. HMR plugins use the transform hook to inject code and depend on module.exports to have all of the exported functions. However, if exports are deferred, the exports will be empty.

Bit stumped on this one. Seems like it'll always be a chicken and egg situation.

high1 commented 4 years ago

I think that this should be documented, and the issue closed - for now. Maybe it'll be solved further down the road, we'll see...

andreaspalsson commented 4 years ago

Looks this issue also affects Sentry. Sentry is using this way of enum declaration for their types like Severity and LogLevel. This causes them to become undefined and Sentry to crash if you call for example Sentry.captureException.

As a workaround you can avoid the crash by not calling Sentry.init() when using Nollup.

Reproduction repo https://github.com/andreaspalsson/nollup-sentry-issue

PepsRyuu commented 4 years ago

Thanks for the repro, much appreciated! It is indeed the same problem. Leave this one with me for a while, I have a few ideas I want to experiment with to try and address this.

PepsRyuu commented 4 years ago

An idea I'm thinking is conditional live bindings. So rather than having live-bindings everywhere, detect if an export is actually exporting anything and if it's not, look for an assignment and then inject an export straight after it. In theory it should work and also keep good performance.

PepsRyuu commented 4 years ago

Here's a prototype that addresses this specific issue. Would appreciate feedback! :)

https://github.com/PepsRyuu/nollup/pull/126

andreaspalsson commented 4 years ago

Wow, thanks for the fast fix. I will test it out first thing on Monday.

high1 commented 4 years ago

The fix works for the repo in the original post, great. Thanks. Do I close this now, or wait for you to merge the branch? Also, does this remove live bindings limitation for nollup?

matt-psaltis commented 4 years ago

Worked for me too! Can't believe the timing just moved my Svelte app to nollup today for better HMR, hit this issue and the timing on the fix couldn't have been better! Thank you!!!

PepsRyuu commented 4 years ago

Brilliant, I'll do a bit more testing with it, and then release before the end of tomorrow. 😊

PepsRyuu commented 4 years ago

@high1 Missed your question there, apologies! Unfortunately no, the live-binding limitation is still there. This is very similar to how circular dependencies is being handled, it addresses a very specific use case. If the export is modified after the module has been initialised, those changes won't be reflected.

PepsRyuu commented 4 years ago

Released in 0.13.3. :)

andreaspalsson commented 4 years ago

Working great in my project as well. Thanks for the fast feedback and fix.

archived-m commented 4 years ago

Can confirm this fixed the issue for us too

kapilpipaliya commented 4 years ago

when i added "targets": { "esmodules": true }on .babelrc "@babel/preset-env" option. import still not work.

PepsRyuu commented 4 years ago

@kapilpipaliya Not sure how that option in Babel would affect this. Do you have a reproducible example?