Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support for UI5-converted classes with class decorators #23

Closed
Elberet opened this issue Mar 28, 2019 · 6 comments · Fixed by #100
Closed

Support for UI5-converted classes with class decorators #23

Elberet opened this issue Mar 28, 2019 · 6 comments · Fixed by #100
Assignees

Comments

@Elberet
Copy link

Elberet commented Mar 28, 2019

I'm working on a concise, easily readable API for class mixins, such as this:

import Controller from "sap/ui/core/mvc/Controller"
import { mixin } from "../lib/decorators"
import RoutingSupport from "../lib/RoutingSupport"

@namespace("example.controller")
@mixin(RoutingSupport)
export default class AppController extends Controller { /* ... */ }

mixin is a simple wrapper that returns a decorator function; the decorator function should eventually be called by Babel's decorators API and receives a descriptor which it uses to register a finisher. The finisher function then gets called with the class' constructor function as its argument and... well, this part is still very much WIP. 😊

Unfortunately, babel-plugin-transform-classes-ui5 doesn't support the very first step: while it respects the @namespace decorator and triggers UI5-transformation for the class, the @mixin decorators are ignored and Babel's decorators API is not invoked.

Any chance to get this working?


FWIW, Babel desugars a decorated class into something like this:

var TheClass = _decorate([mixin], function(_initialize, _BaseClass) {
  var TheClass = function(_BaseClass2) {
    /* CONSTRUCTOR FACTORY BODY */
  }(_BaseClass);

  return {
    F: TheClass,
    d: [ /* PROPERTY/METHOD DESCRIPTORS */ ]
  };
}, BaseClass);

The first commented-out section is basically the usual class constructor factory, with the big difference being that method and property definitions are moved into the second commented-out section, each wrapped into a descriptor object so that the decorators API can process them further.

This indicates to me that if support for method/property decorators is omitted until some future version, supporting just class decorators should™ be fairly straight forward - just pull in Babel's decorators machinery and feed it the constructor function returned by UI5's extend...

@lucasheim
Copy link

@Elberet I know this is old, but are you using @babel/plugin-proposal-decorators in your babel pipeline? In my project, we have custom decorators too and things worked out when adding this step.

@Elberet
Copy link
Author

Elberet commented Nov 20, 2019

Well, sometime during the past half year, I've migrated away from this plugin towards a custom set of Babel plugins... but form what I can recall:

Yes, @babel/plugin-proposal-decorators was present in my Babel config, but from what I gleaned from its code, babel-plugin-transform-modules-ui5 would completely replace the ClassDeclaration node with the UI5 class creation logic long before the standard Babel plugin handling classes would get a chance to visit the node and decide, based on feature flags enabled by proposal-decorators, to create the class using the decorators API.

And, since babel-plugin-transform-modules-ui5 does all this in a Program enter visitor, plugin ordering seemed largely irrelevant because that node is always visited before any ClassDeclarations the file might contain, so I gave up trying to get things to work, opened this issue and later decided to just learn the Babel API and roll my own.

That also allowed me to do a few other things right in my build pipeline, such as allowing imports of NPM modules that get automatically webpacked into a vendor-bundle, and building a runtime-bundle from latest core-js and regenerator-runtime instead of using the deprecated @babel/polyfill library... so I have no regrets. 😉

@lucasheim
Copy link

Thanks for the insights @Elberet! Do you have any available examples of your final build pipeline? I'm currently looking into alternatives to work with TS + UI5.

@Elberet
Copy link
Author

Elberet commented Nov 20, 2019

I wrote the thing on company time, so I'll have to check if I can put it under some OS license and put it on GitHub, but the chances of that happening are unfortunately slim to none. 😞

If you would like to follow in my footsteps, I can give some pointers tho, but it pretty much boils down to: be frugal, let other plugins do as much as possible, and keep in mind that you can always use path.traverse to walk through the syntax tree without other plugins messing things up; in fact, nesting traverse calls is almost always a better idea than to attempt storing and consuming state information in node handlers within the same visitor.

@ArnaudBuchholz
Copy link

Hello, I see the issue is still open and I need decorators to achieve some special logic with a JSONModel.

I observe that my decorators are not being converted to JavaScript. Since I am not familiar with babel, I searched and found that I needed to add @babel/plugin-proposal-decorators, @babel/plugin-proposal-class-properties
and @babel/plugin-transform-flow-strip-types which I did but... unsuccessfully.

It is supposed to work ?

@Elberet
Copy link
Author

Elberet commented Dec 5, 2021

No, transform-modules-ui5 (this plugin) interferes with Babel's handling of decorators. Changing this would require a complete redesign of this plugin.

As mentioned above, by the time @babel/plugin-proposal-decorators runs, transform-modules-ui5 has already stripped the source tree of all classes, methods and properties and their associated decorators, and has replaced them with UI5-style class declarations. Other plugins - such as @babel/plugin-proposal-decorators - will only see a function call declaration for BaseClass.extend() and an inline object declaration. Thus, the bulk of Babel's code responsible for emitting transpiled classes and runtime decorator-calls is never executed.

petermuessig added a commit that referenced this issue May 29, 2023
To ensure that other plugins can be used upfront the conversion of
the ES6 imports and classes to UI5 AMD-like syntax and Object.extend,
the plugin now runs in the exit phase of the visitor. This allows
other plugins, e.g. decorators to run before and convert the code
accordingly. This plugin only sanitizes the decorators to remove the
plugin proprietary ones to avoid conflicts with other plugins.

Fixes #23
petermuessig added a commit that referenced this issue May 29, 2023
To ensure that other plugins can be used upfront the conversion of
the ES6 imports and classes to UI5 AMD-like syntax and Object.extend,
the plugin now runs in the exit phase of the visitor. This allows
other plugins, e.g. decorators to run before and convert the code
accordingly. This plugin only sanitizes the decorators to remove the
plugin proprietary ones to avoid conflicts with other plugins.

Fixes #23

fix: update plugin dependencies
petermuessig added a commit that referenced this issue May 29, 2023
To ensure that other plugins can be used upfront the conversion of
the ES6 imports and classes to UI5 AMD-like syntax and Object.extend,
the plugin now runs in the exit phase of the visitor. This allows
other plugins, e.g. decorators to run before and convert the code
accordingly. This plugin only sanitizes the decorators to remove the
plugin proprietary ones to avoid conflicts with other plugins.

The plugin is now also not dependent on the plugin order anymore and
using the preset-env ensures a proper transpilation of code to a
specific target browser set.

Added tests to ensure that a proper decorator handling works when
using the plugin-proposal-decorators or that e.g the conversion of
getters/setters takes place when using the babel plugin named
plugin-transform-property-mutators as this is not part of the
preset-env by default.

Updated all dependencies of the project with this change.

Fixes #23
Fixes #25
@petermuessig petermuessig self-assigned this May 29, 2023
petermuessig added a commit that referenced this issue May 30, 2023
To ensure that other plugins can be used upfront the conversion of
the ES6 imports and classes to UI5 AMD-like syntax and Object.extend,
the plugin now runs in the exit phase of the visitor. This allows
other plugins, e.g. decorators to run before and convert the code
accordingly. This plugin only sanitizes the decorators to remove the
plugin proprietary ones to avoid conflicts with other plugins.

Added tests to ensure that a proper decorator handling works when
using the plugin-proposal-decorators or that e.g the conversion of
getters/setters takes place when using the babel plugin named
plugin-transform-property-mutators as this is not part of the
preset-env by default.

Updated all dependencies of the project with this change.

Fixes #23
Fixes #25
petermuessig added a commit that referenced this issue May 30, 2023
…100)

To ensure that other plugins can be used upfront the conversion of
the ES6 imports and classes to UI5 AMD-like syntax and Object.extend,
the plugin now runs in the exit phase of the visitor. This allows
other plugins, e.g. decorators to run before and convert the code
accordingly. This plugin only sanitizes the decorators to remove the
plugin proprietary ones to avoid conflicts with other plugins.

Added tests to ensure that a proper decorator handling works when
using the plugin-proposal-decorators or that e.g the conversion of
getters/setters takes place when using the babel plugin named
plugin-transform-property-mutators as this is not part of the
preset-env by default.

Updated all dependencies of the project with this change.

Fixes #23
Fixes #25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants