feat: support class properties transformation#361
Conversation
e2650af to
17d7f43
Compare
|
Thank you for the PR and sorry for late review. I have added a dep reproduction and it fixes issue. Testing on some projects, I found effect of this change is that babel is transpiling more than it should (for example private export class User {
#id = 0;
constructor() {
this.#init();
console.log(this.#id);
}
#init() {
this.#id = Math.random();
}
}Was working before without any transform (JS has native private field/method support) but now fails:
Of course we can add more transforms but it is raising a question for me if it is safe thing to do. One way we can do to move this faster is to make transform opt-in on top of syntax. |
Fine by me, but there might be more problems, so maybe things should be tested. |
|
Still, I support the idea of proceeding with opt-in transforms for now so we can address any arising issues and, after some time, merge them as enabled by default 👍 |
|
Hey guys 👋 I've implemented the "experimentalTransforms" option, along with a fixture for typestack packages. You can test what happens without the option by going into I've also fixed the missing MikroOrm fixture import in deps/index.ts |
|
Thank you! Perhaps better call it more explicitly |
|
I would advocate for the Also, I don't think people should enable this option unless they got a real reason for it, like these edge cases with typestack and mikro-orm. I may be wrong, let me know 🧐 |
|
I think we would never enable this by default, as it implies more transformation and less proximity to runtime native behavior . |
Does that mean that jiti's not aiming to be compatible with tsc as much as possible? Because this specific feature should improve said compatibility. |
|
Yes generic opt-in flag is cool 👍🏼 (my goal for jiti is to be as close to native Node.js as possible by default.) |
So, |
|
Jiti supports all of those (TS features). I meant runtime (JS) syntax support like class props. It can be easily an opt in feature for when needed for class props :) |
|
@pi0 Done. If it ain't going to be default anywhere in the future, then transformClassProps seems right. |
Resolves
This PR resolves Originally posted by @cshomo11 in #57
MWE
MWE (minimal reproduction) repository
For testing the MWE, just hit
pnpm installand thenpnpm workingorpnpm not-working.Description
We should not be using the
babel-plugin-syntax-class-properties:Just as written in the babel docs, the plugin does not bring transform, so it causes an issue with
class-transfomer(and probably other packages):The only fix without merging is including the plugin by yourself:
Transpiled code that failed