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

[RFC] 0012 UI5 Tooling Extension API v3 #664

Merged
merged 11 commits into from
Nov 22, 2023

Conversation

RandomByte
Copy link
Member

@RandomByte RandomByte commented Oct 31, 2022

Request for comment summarizing potential enhancements to APIs provided to UI5 Tooling extensions. Namely custom tasks and custom middleware.

View rendered version (main branch)

@RandomByte RandomByte force-pushed the rfc-ui5-tooling-extension-api-v3 branch 2 times, most recently from 770da5a to 3f49acc Compare November 2, 2022 12:52
@RandomByte RandomByte marked this pull request as ready for review November 2, 2022 12:55
@RandomByte RandomByte force-pushed the rfc-ui5-tooling-extension-api-v3 branch from 3f49acc to 68c725a Compare November 2, 2022 14:12
@vobu
Copy link

vobu commented Nov 3, 2022

i like many -if not all- of the proposed enhancement! Also, introducing a breaking change w/ v3 is fine from my POV as providing backwards compatibility at that point would mean too much technical debt to maintain.

That being said, I'm somewhat torn on the ESM-only approach of providing the ui5 tooling modules themselves as this forces migration of all tasks and middlewares requiring any @ui5/* modules. The CJS/ESM split has caused enough uproar in the Node-community already and we shouldn't amplify this in UI5-land. Thus, I'd suggest to provide both ESM- and CJS-modules, similar to how fastify is doing it; and let the users decide on what to use.

What I'd like to also see is some improvement on bundling size - ui5 build all produces huge bundles that from my admittedly naïve POV could be shaken down significantly 😸

@flovogt flovogt changed the base branch from master to v2 November 3, 2022 13:45
@flovogt flovogt changed the base branch from v2 to main November 3, 2022 13:46
@RandomByte RandomByte force-pushed the rfc-ui5-tooling-extension-api-v3 branch from 68c725a to a8e666d Compare November 3, 2022 14:02
@mauriciolauffer
Copy link

I agree with @vobu , especially on the build comment. Today, "tree shaking" ignores sap.ui.core and shakes just other modules.

@mauriciolauffer
Copy link

I'd love to see the end of ui5 in package.json.

"ui5": { "dependencies": [ "no-more!" ] }

@RandomByte
Copy link
Member Author

RandomByte commented Nov 4, 2022

Thank you both for your feedback! Much appreciated!

@vobu: Our motivation to move to ESM mainly stems from the fact that many of our dependencies already made the move. This lead to situations where we struggled to consume security patches in dependencies. But we also like the concept in general, as well as the great interoperability with CommonJS. We think the Node.js ecosystem will move towards ESM more and more.

I'm somewhat torn on the ESM-only approach of providing the ui5 tooling modules themselves as this forces migration of all tasks and middlewares requiring any @ui5/* modules

We actually do not expect any great efforts for extensions that migrate to our v3 modules. When lifting the dependency, all they should need to do is replace for example require("@ui5/builder").xyz with await import("@ui5/builder/xyz"). There is no need to migrate the extension as a whole to ESM, thanks to dynamic imports being available in CommonJS too. Or is there something else that comes with this and causes concern for you?

Thus, I'd suggest to provide both ESM- and CJS-modules, similar to how fastify is doing it

Maybe Fastify is not the best example for our case, since the library itself is still written in CommonJS. It does not provide ES-modules in addition to CommonJS modules, but merely "ESM-syle" export of the same CommonJS modules.

When trying to require() our ES-modules on the other hand, Node.js will not allow this and throw an error. The only way to work around this, would be to transpile all relevant modules to CommonJS and ship both variants. Similar to how for example Yargs does it.

What I'd like to also see is some improvement on bundling size

That's still a major topic for us and many UI5 devs are looking into it. While it doesn't seem likely to get major improvements in UI5 Tooling v3, we got some related features like source maps for bundles and faster bundling in general.

I'd love to see the end of ui5 in package.json.

That's already part of the current v3 alpha release. If nobody faces any trouble during the beta, this will be reality soon 👍

@ecker ecker added the RFC Request for Comment (pull request) label Nov 14, 2022
@RandomByte RandomByte force-pushed the rfc-ui5-tooling-extension-api-v3 branch from b5c2611 to 209662b Compare November 18, 2022 10:02
RandomByte added a commit to SAP/ui5-server that referenced this pull request Dec 2, 2022
…source

For custom middleware this will returned a Specification
Version-dependent (=compatible) interface of the Project instance a
Resource is associated with.

This was already described in RFC12[1] but not part of the initial
implementation of MiddlewareUtil#getProject.

[1] SAP/ui5-tooling#664
RandomByte added a commit to SAP/ui5-project that referenced this pull request Dec 2, 2022
For custom task, this will returned a Specification Version-dependent
(=compatible) interface of the Project instance a Resource is associated
with.

This was already described in RFC12[1] but not part of the initial
implementation of TaskUtil#getProject.

[1] SAP/ui5-tooling#664
RandomByte added a commit to SAP/ui5-project that referenced this pull request Dec 2, 2022
For custom task, this will return a Specification Version-dependent
(=compatible) interface of the Project instance a Resource is associated
with.

This was already described in RFC12[1] but not part of the initial
implementation of TaskUtil#getProject.

[1] SAP/ui5-tooling#664
RandomByte added a commit to SAP/ui5-server that referenced this pull request Dec 2, 2022
…source

For custom middleware this will return a Specification
Version-dependent (=compatible) interface of the Project instance a
Resource is associated with.

This was already described in RFC12[1] but not part of the initial
implementation of MiddlewareUtil#getProject.

[1] SAP/ui5-tooling#664
RandomByte added a commit to SAP/ui5-server that referenced this pull request Dec 7, 2022
…source

For custom middleware this will return a Specification
Version-dependent (=compatible) interface of the Project instance a
Resource is associated with.

This was already described in RFC12[1] but not part of the initial
implementation of MiddlewareUtil#getProject.

[1] SAP/ui5-tooling#664
This allows to provide other helpful functions too, like
"createFilterReader"
* Replace getResourceFactory() function with a resourceFactory
  property
* Replace getLogger function with a log property passed directly to the
  task/middleware (not using the corresponding -util)
With the latest changes in SAP/ui5-project#431,
the method now returns an instance of a "SpecificationVersion" class.
This would require an additional specVersion-dependent interface.

Since use cases in custom tasks and -middelare are not clear, we should
rather keep it simple for now and do not provide this method.
@RandomByte RandomByte force-pushed the rfc-ui5-tooling-extension-api-v3 branch from c978671 to 083085f Compare January 24, 2023 13:25
RandomByte added a commit to SAP/ui5-project that referenced this pull request Jan 24, 2023
As proposed in chapter 6 of RFC0012[1], restrict the 'name' property for
projects and extensions defining specification version 3.0 and later.

BREAKING CHANGE:

For projects and extensions defining specVersion 3.0 and later, the
metadata.name property must satisfy the following conditions:

* Must be at least 3 characters long
* Must be no longer than 50 characters
* Must only contain lowercase characters
* Must only contain alphanumeric characters, dash, underscore, dot
    - Exception: `@` and `/` are allowed at certain positions as
      explained below
* Must start with an alphabetic character or an `@`-character
* If a name starts with an `@`-character, it must contain exactly one
  forward-slash `/`
    - This is aligned with the npm concept for package scopes
    - e.g. `@org/lib.name`

For details, please see refer to chapter 6 of RFC0012[1].

[1]: SAP/ui5-tooling#664
RandomByte added a commit to SAP/ui5-project that referenced this pull request Jan 24, 2023
As proposed in chapter 6 of RFC0012[1], restrict the 'name' property for
projects and extensions defining specification version 3.0 and later.

BREAKING CHANGE:

For projects and extensions defining specVersion 3.0 and later, the
metadata.name property must satisfy the following conditions:

* Must be at least 3 characters long
* Must be no longer than 50 characters
* Must only contain lowercase characters
* Must only contain alphanumeric characters, dash, underscore, dot
    - Exception: `@` and `/` are allowed at certain positions as
      explained below
* Must start with an alphabetic character or an `@`-character
* If a name starts with an `@`-character, it must contain exactly one
  forward-slash `/`
    - This is aligned with the npm concept for package scopes
    - e.g. `@org/lib.name`

For details, please see refer to chapter 6 of RFC0012[1].

[1]: SAP/ui5-tooling#664
RandomByte added a commit to SAP/ui5-project that referenced this pull request Jan 24, 2023
As proposed in chapter 6 of RFC0012([1]), restrict the 'name' property for
projects and extensions defining specification version 3.0 and later.

BREAKING CHANGE:

For projects and extensions defining specVersion 3.0 and later, the
metadata.name property must satisfy the following conditions:

* Must be at least 3 characters long
* Must be no longer than 50 characters
* Must only contain lowercase characters
* Must only contain alphanumeric characters, dash, underscore, dot
    - Exception: `@` and `/` are allowed at certain positions as
      explained below
* Must start with an alphabetic character or an `@`-character
* If a name starts with an `@`-character, it must contain exactly one
  forward-slash `/`
    - This is aligned with the npm concept for package scopes
    - e.g. `@org/lib.name`

For details, please see refer to chapter 6 of RFC0012([1]).

[1]: SAP/ui5-tooling#664
@RandomByte RandomByte force-pushed the rfc-ui5-tooling-extension-api-v3 branch from 083085f to 6ee70e3 Compare January 24, 2023 16:57
RandomByte added a commit to SAP/ui5-project that referenced this pull request Jan 24, 2023
As proposed in chapter 6 of RFC0012([1]), restrict the 'name' property for
projects and extensions defining specification version 3.0 and later.

BREAKING CHANGE:

For projects and extensions defining specVersion 3.0 and later, the
metadata.name property must satisfy the following conditions:

* Must be at least 3 characters long
* Must be no longer than 50 characters
* Must contain lowercase characters only
* Must contain alphanumeric characters, dash, underscore and period only
    - Exception: `@` and `/` are allowed at certain positions as
      explained below
* Must start with an alphabetic character or an `@`-character
* If a name starts with an `@`-character, it must contain exactly one
  forward-slash `/`
    - This is aligned with the npm concept for package scopes
    - e.g. `@org/lib.name`

For details, please see refer to chapter 6 of RFC0012([1]).

[1]: SAP/ui5-tooling#664
RandomByte added a commit to SAP/ui5-project that referenced this pull request Jan 24, 2023
As proposed in chapter 6 of RFC0012([1]), restrict the 'name' property for
projects and extensions defining specification version 3.0 and later.

BREAKING CHANGE:

For projects and extensions defining specVersion 3.0 and later, the
metadata.name property must satisfy the following conditions:

* Must be at least 3 characters long
* Must be no longer than 50 characters
* Must contain lowercase characters only
* Must contain alphanumeric characters, dash, underscore and period only
    - Exception: `@` and `/` are allowed at certain positions as
      explained below
* Must start with an alphabetic character or an `@`-character
* If a name starts with an `@`-character, it must contain exactly one
  forward-slash `/`
    - This is aligned with the npm concept for package scopes
    - e.g. `@org/lib.name`

For details, please see refer to chapter 6 of RFC0012([1]).

[1]: SAP/ui5-tooling#664
RandomByte added a commit to SAP/ui5-project that referenced this pull request Jan 25, 2023
As proposed in chapter 6 of RFC0012([1]), restrict the 'name' property for
projects and extensions defining specification version 3.0 and later.

BREAKING CHANGE:

For projects and extensions defining specVersion 3.0 and later, the
metadata.name property must satisfy the following conditions:

* Must be at least 3 characters long
* Must be no longer than 50 characters
* Must contain lowercase characters only
* Must contain alphanumeric characters, dash, underscore and period only
    - Exception: `@` and `/` are allowed at certain positions as
      explained below
* Must start with an alphabetic character or an `@`-character
* If a name starts with an `@`-character, it must contain exactly one
  forward-slash `/`
    - This is aligned with the npm concept for package scopes
    - e.g. `@org/lib.name`

For details, please see refer to chapter 6 of RFC0012([1]).

[1]: SAP/ui5-tooling#664
RandomByte added a commit to SAP/ui5-project that referenced this pull request Jan 27, 2023
As proposed in chapter 6 of RFC0012([1]), restrict the 'name' property for
projects and extensions defining specification version 3.0 and later.

BREAKING CHANGE:

For projects and extensions defining specVersion 3.0 and later, the
metadata.name property must satisfy the following conditions:

* Must be at least 3 characters long
* Must be no longer than 50 characters
* Must contain lowercase characters only
* Must contain alphanumeric characters, dash, underscore and period only
    - Exception: `@` and `/` are allowed at certain positions as
      explained below
* Must start with an alphabetic character or an `@`-character
* If a name starts with an `@`-character, it must contain exactly one
  forward-slash `/`
    - This is aligned with the npm concept for package scopes
    - e.g. `@org/lib.name`

For details, please see refer to chapter 6 of RFC0012([1]).

[1]: SAP/ui5-tooling#664
d3xter666 pushed a commit to SAP/ui5-project that referenced this pull request Feb 7, 2023
As proposed in chapter 6 of RFC0012([1]), restrict the 'name' property for
projects and extensions defining specification version 3.0 and later.

BREAKING CHANGE:

For projects and extensions defining specVersion 3.0 and later, the
metadata.name property must satisfy the following conditions:

* Must be at least 3 characters long
* Must be no longer than 50 characters
* Must contain lowercase characters only
* Must contain alphanumeric characters, dash, underscore and period only
    - Exception: `@` and `/` are allowed at certain positions as
      explained below
* Must start with an alphabetic character or an `@`-character
* If a name starts with an `@`-character, it must contain exactly one
  forward-slash `/`
    - This is aligned with the npm concept for package scopes
    - e.g. `@org/lib.name`

For details, please see refer to chapter 6 of RFC0012([1]).

[1]: SAP/ui5-tooling#664
flovogt
flovogt previously approved these changes Jun 27, 2023
Making this PR ready to be merged
@RandomByte RandomByte merged commit 8291611 into main Nov 22, 2023
7 checks passed
@RandomByte RandomByte deleted the rfc-ui5-tooling-extension-api-v3 branch November 22, 2023 11:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
RFC Request for Comment (pull request)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants