Fix decorators interaction with abstract/declare fields#17947
Conversation
nicolo-ribaudo
added
the
PR: Bug Fix 🐛
|
Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/61364 |
|
commit: |
| if (kind === FIELD_IGNORED) { | ||
| if (hasDecorators) { | ||
| throw element.buildCodeFrameError( | ||
| "Class elements marked with 'declare' or 'abstract' cannot be decorated.", |
There was a problem hiding this comment.
I think this error should be moved to the parser. TSC forbids such errors, too:
JLHwung
left a comment
JLHwung
left a comment
There was a problem hiding this comment.
It seems to me that this PR does not quite fix the original issue.
To recap, we injected the protoInitCall, one of the fieldInitializerExpressions to a non-static field:
babel/packages/babel-helper-create-class-features-plugin/src/decorators.ts
Lines 1539 to 1549 in 508caa2
The original issue does not provide the config required to reproduce the error. I tried a bit on REPL and I can only reproduce the error when we keep the typescript syntax but transform the decorators (link) And the posted error stacktrace does look like a third-party TS parser, which means the user might have kept the TS syntax and fed the Babel output to another TS aware tool.
Previously we disallowed untransformed TS class features in packages/babel-helper-create-class-features-plugin/src/typescript.ts, maybe we can guard the abstract field as well. Alternatively, we can also skip TS abstract field / declared field / finite field in the decorators transform.
|
This PR fixes the case in which you only transpile decorators and not TS. I suspect that the original issue is using Babel to transpile decorators, and then feeding the output into Oxc to transpile the rest.
Uhm, isn't this what this PR does? We skip abstract/declare fields in the loop so that we don't attach the I'll split the parser changes into a separate PR: #17949 |
Thank you, yeah the split changes are much easier to review. |
4 participants
Fixes #1, Fixes #2Fixes #17942