feat: support class properties transformation
Resolves
This PR resolves Originally posted by @cshomo11 in #57
MWE
MWE (minimal reproduction) repository
For testing the MWE, just hit pnpm install and then pnpm working or pnpm 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):
[jiti] [init] version: 2.4.2 module-cache: true fs-cache: false interop-defaults: true
[jiti] [transpile] [esm] ./src/class-validator/env.config.ts (51.721ms)
[jiti] [native] [import] workspace/my-lib/packages/my-lib/dist/core/core.cjs
[jiti] [native] [import] workspace/my-lib/node_modules/.pnpm/[email protected]/node_modules/dotenv/lib/main.js
[jiti] [native] [import] workspace/my-lib/node_modules/.pnpm/[email protected]/node_modules/class-validator/cjs/index.js
[jiti] [native] [import] workspace/my-lib/node_modules/.pnpm/[email protected]/node_modules/class-transformer/cjs/index.js
workspace/my-lib/packages/playground/src/class-validator/env.config.ts:7
<TRANSPILED CODE THAT FAILED GOES HERE>
Error: Decorating class property failed. Please ensure that transform-class-properties is enabled and runs after the decorators transform.
at _initializerWarningHelper (workspace/my-lib/packages/playground/src/class-validator/env.config.ts:7:767)
at <instance_members_initializer> (workspace/my-lib/packages/playground/src/class-validator/env.config.ts:13:141)
at new Schema (workspace/my-lib/packages/playground/src/class-validator/env.config.ts:13:116)
at TransformOperationExecutor.transform (workspace/my-lib/node_modules/.pnpm/[email protected]/node_modules/class-transformer/cjs/TransformOperationExecutor.js:147:32)
at ClassTransformer.plainToInstance (workspace/my-lib/node_modules/.pnpm/[email protected]/node_modules/class-transformer/cjs/ClassTransformer.js:27:25)
at plainToInstance (workspace/my-lib/node_modules/.pnpm/[email protected]/node_modules/class-transformer/cjs/index.js:38:29)
at validate (workspace/my-lib/packages/playground/src/class-validator/env.config.ts:27:60)
at Command.<anonymous> (workspace/my-lib/packages/my-lib/dist/cli/cli.cjs:334:39)
Node.js v22.3.0
ELIFECYCLE Command failed with exit code 1.
The only fix without merging is including the plugin by yourself:
import pluginTransformClassProperties from "@babel/plugin-transform-class-properties";
const jiti = createJiti(import.meta.url, {
transformOptions: {
ts: true,
babel: {
plugins: [pluginTransformClassProperties],
},
},
});
Transpiled code that failed
var _classTransformer = await jitiImport("class-transformer");
var _dec, _dec2, _dec3, _dec4, _class, _descriptor, _descriptor2;
function _interopRequireDefault(e) {
return e && e.__esModule ? e : { default: e };
}
function _applyDecoratedDescriptor(i, e, r, n, l) {
var a = {};
return (
Object.keys(n).forEach(function (i) {
a[i] = n[i];
}),
(a.enumerable = !!a.enumerable),
(a.configurable = !!a.configurable),
("value" in a || a.initializer) && (a.writable = !0),
(a = r
.slice()
.reverse()
.reduce(function (r, n) {
return n(i, e, r) || r;
}, a)),
l && void 0 !== a.initializer && ((a.value = a.initializer ? a.initializer.call(l) : void 0), (a.initializer = void 0)),
void 0 === a.initializer ? (Object.defineProperty(i, e, a), null) : a
);
}
function _initializerWarningHelper(r, e) {
throw Error("Decorating class property failed. Please ensure that transform-class-properties is enabled and runs after the decorators transform.");
}
let;
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:
[Error: TRANSFORM_ERROR: Class private methods are not enabled. Please add
@babel/plugin-transform-private-methodsto your configuration.
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.
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.
plugin-transform-class-properties only deals with classes, so every other transform we may need will probably be class-related. Therefore, I don't think we will need many more transforms.
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 fixtures.test.ts and setting JITI_EXPERIMENTAL_TRANSFORMS: "false".
I've also fixed the missing MikroOrm fixture import in deps/index.ts
Thank you! Perhaps better call it more explicitly transformClassProps ?
I would advocate for the experimental keyword if we were to make them default someday. We may need to add other transforms beyond the current two, so who knows if all experimental transforms will be related to transforming class properties?
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 .
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. I think if not, maybe it should be opt-in under more generic flag, of course if there's more things to improve there, aside from class transformations.
Yes generic opt-in flag is cool 👍🏼 (my goal for jiti is to be as close to native Node.js as possible by default.)
my goal for jiti is to be as close to native Node.js as possible by default
So, erasableSyntaxOnly + rewriteRelativeImportExtensions + allowImportingTsExtensions compatibility by default, got it. Maybe it needs to be mentioned in readme somewhere? Because I don't really see it, but maybe it's just me :D
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.