Ensure class decorators can access decorated non-static members by JLHwung · Pull Request #15332 · babel/babel

JLHwung · 2023-01-06T20:29:37Z

Q	A
Fixed Issues?	Fixes #15325
Patch: Bug Fix?	Yes
Major: Breaking Change?
Minor: New Feature?
Tests Added + Pass?	Yes
Documentation PR Link
Any Dependency Changes?
License	MIT

Introduce a new runtime helper applyDecs2203R based on applyDecs2203, which lazily apply class decorations so that the class decorate will have access to the decorated non-static class elements. Because the return type is changed, we have to create a new runtime. In Babel 8 we can remove deprecated helpers applyDecs2203 and applyDecs
Optimize the output when only the class is decorated or elements are decorated

This PR slightly increases the output size:

// before
[_init_a, _initProto] = babelHelpers.applyDecs2203(Foo, [[dec, 1, "a"]], []);
// after
[_init_a, _initProto] = babelHelpers.applyDecs2203R(Foo, [[dec, 1, "a"]], []).e;

We could preserve the output size if applyDecs2203R can return a customized iterator, i.e.

yield* applyMemberDecs(targetClass, memberDecs);
yield* applyClassDecs(targetClass, classDecs);

but then our runtime helper will not be ES5-compliant. We can revisit this approach if we bump helper targets in the future.

JLHwung · 2023-01-06T20:31:22Z

packages/babel-helpers/src/helpers/applyDecs2203R.js

+    get c() {
+      return applyClassDecs(targetClass, classDecs);
+    },


The class decorators are applied after member decorators are applied and bound to the initializer locals, such as initA, initProto, etc.

babel-bot · 2023-01-06T20:32:51Z

Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/53767/

packages/babel-helpers/src/helpers/applyDecs2203.js

liuxingbaoyu · 2023-01-08T19:36:02Z

The CI failure looks related.

JLHwung · 2023-01-09T16:39:44Z

Yes. CI is failing for the same reason of #14836 (comment). I will investigate how I can resolve it without renaming functions.

nicolo-ribaudo · 2023-01-09T17:03:34Z

Helper diff: https://www.diffchecker.com/wMz5rhsx/

JLHwung · 2023-01-09T17:41:16Z

The e2e build was failing because we simply concat all helper codes in build-external-helpers:

babel/packages/babel-core/src/tools/build-external-helpers.ts

Lines 172 to 181 in 6e03af1

    
           helpers.list.forEach(function (name) { 
        
             if (allowlist && allowlist.indexOf(name) < 0) return; 
        
             const ref = (refs[name] = getHelperReference(name)); 
        
             helpers.ensure(name, File); 
        
             const { nodes } = helpers.get(name, getHelperReference, ref); 
        
             body.push(...nodes); 
        
           });

which forms the content of the virtual \u0000RollupPluginBabelHelpers module created by the rollup Babel plugin.

A proper fix would be to handle function name collisions in build-external-helpers, but given that in most cases, the function names collide because we intend to replace the runtime with a new version. With the old runtime deprecated and hopefully removed in the next major, I think wrapping it into a factory is sufficient.

nicolo-ribaudo

@JLHwung The Attempted to decorate a public method/accessor that has the same name as a previously decorated public method/accessor. This is not currently supported by the decorators plugin. Property name was: error has been removed from the new helper: is it now supported, or was that error just unreachable?

This PR slightly increases the output size:

It's fine, decorators are already considered heavy and this is just a few more bytes.

JLHwung · 2023-01-09T19:36:57Z

The Attempted to decorate a public method/accessor that has the same name as a previously decorated public method/accessor. This is not currently supported by the decorators plugin. Property name was: error has been removed from the new helper

That part is not changed:

5dbfd3d#diff-1055897236adecf62778dc76b5b6168bc5c14344106b32a8a14c78479f601cb2R397

The only revisions are that applyMemberDecs and applyClassDecs now return the ret array, and the applyDecs returns an object served as a 2-item iterator. I can rewrite the commit history so we can easily review the difference.

packages/babel-plugin-proposal-decorators/src/transformer-2022-03.ts

…-03.ts Co-authored-by: Nicolò Ribaudo <nicolo.ribaudo@gmail.com>

JLHwung added PR: Bug Fix 🐛 A type of pull request used for our changelog categories Spec: Decorators labels Jan 6, 2023

JLHwung commented Jan 6, 2023

View reviewed changes

JLHwung force-pushed the fix-15325 branch from 45d7f59 to 1e6f0c6 Compare January 6, 2023 20:34

liuxingbaoyu reviewed Jan 7, 2023

View reviewed changes

packages/babel-helpers/src/helpers/applyDecs2203.js Outdated Show resolved Hide resolved

nicolo-ribaudo reviewed Jan 9, 2023

View reviewed changes

copy applyDecs2203 to applyDecs2203R

ef9e29c

JLHwung force-pushed the fix-15325 branch from 68eacb7 to 5138bfb Compare January 9, 2023 19:43

JLHwung added 5 commits January 9, 2023 15:10

Introduce applyDecs2203R

5fcbcf0

update test fixtures

90c9b34

make node < 16.11 happy

3368ba2

Update packages/babel-helpers/src/helpers/applyDecs2203.js

2097978

wrap applyDecs2203 impl into a factory

2c86567

JLHwung force-pushed the fix-15325 branch from 5138bfb to 2c86567 Compare January 9, 2023 20:10

nicolo-ribaudo approved these changes Jan 12, 2023

View reviewed changes

packages/babel-plugin-proposal-decorators/src/transformer-2022-03.ts Show resolved Hide resolved

Update packages/babel-plugin-proposal-decorators/src/transformer-2022…

2b8b8e2

…-03.ts Co-authored-by: Nicolò Ribaudo <nicolo.ribaudo@gmail.com>

liuxingbaoyu approved these changes Jan 18, 2023

View reviewed changes

JLHwung merged commit 0f68471 into babel:main Jan 18, 2023

JLHwung deleted the fix-15325 branch January 18, 2023 02:27

github-actions bot added the outdated A closed issue/PR that is archived due to age. Recommended to make a new issue label Apr 20, 2023

github-actions bot locked as resolved and limited conversation to collaborators Apr 20, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Ensure class decorators can access decorated non-static members#15332

Ensure class decorators can access decorated non-static members#15332
JLHwung merged 7 commits intobabel:mainfrom
JLHwung:fix-15325

JLHwung commented Jan 6, 2023 •

edited by gitpod-io bot

Loading

Uh oh!

JLHwung Jan 6, 2023

Uh oh!

babel-bot commented Jan 6, 2023 •

edited

Loading

Uh oh!

Uh oh!

liuxingbaoyu commented Jan 8, 2023

Uh oh!

JLHwung commented Jan 9, 2023

Uh oh!

nicolo-ribaudo commented Jan 9, 2023

Uh oh!

JLHwung commented Jan 9, 2023

Uh oh!

nicolo-ribaudo left a comment •

edited

Loading

Uh oh!

JLHwung commented Jan 9, 2023

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Uh oh!

Conversation

JLHwung commented Jan 6, 2023 • edited by gitpod-io bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

JLHwung Jan 6, 2023

Choose a reason for hiding this comment

Uh oh!

babel-bot commented Jan 6, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

liuxingbaoyu commented Jan 8, 2023

Uh oh!

JLHwung commented Jan 9, 2023

Uh oh!

nicolo-ribaudo commented Jan 9, 2023

Uh oh!

JLHwung commented Jan 9, 2023

Uh oh!

nicolo-ribaudo left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JLHwung commented Jan 9, 2023

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

JLHwung commented Jan 6, 2023 •

edited by gitpod-io bot

Loading

babel-bot commented Jan 6, 2023 •

edited

Loading

nicolo-ribaudo left a comment •

edited

Loading