chore!: drop ast check for refresh boundary#43
Conversation
|
I think I'm okay with this, but @aleclarson suggested to keep the AST check in place when I was adding the runtime check. So maybe he has some thoughts about this. Ref: https://discord.com/channels/804011606160703521/1018203348060618782/1019332955551834162 |
The AST check avoids an unnecessary runtime check and code transformation in many cases. I haven't measured the performance impact of removing the AST check, so I can't help you decide whether merging this is the best option or not. |
|
I think it is a good idea to align with other implementations. But let's remove this once we know the perf implications. |
|
I can do some perf checks but I disagree with:
The ast check is always done after code transformation, so you can't skip transformation. It's done only when babel already added some "Refrehreg" code, so in >>90% of the cases for codebase that respect rules (and you should otherwise HMR is a nightmare), you will run both AST and runtime checks that both says everything is valid. |
The runtime only check is closer to what other implementation are doing including the vite SWC one.
This is mostly for simplification and consistency between plugins. It will also have the benefit of allowing people to know when the fast refresh was skipped when I will port the invalidate message from the SWC plugin.
cc @IanVS