feat: PoC: node modules & webcomponents integration#1287
feat: PoC: node modules & webcomponents integration#1287
Conversation
d3xter666
force-pushed
the
poc-rfc-modules-integration
branch
from
7d0b336 to
480d3e0
Compare
rfcs/0020-npm-package-integration.md
Outdated
| - **Dev-Time Support** — On-demand bundling during `ui5 serve` (deferred to [RFC 0017 Incremental Build](https://github.com/UI5/cli/blob/rfc-incremental-build/rfcs/0017-incremental-build.md)) | ||
| - **Standard Compliance** — Output follows UI5's AMD module format and naming conventions | ||
| - **Web Component Support** — First-class `@ui5/webcomponents` integration with UI5 control wrappers | ||
| - **Performance Optimization** — Package independence enables deduplication and browser caching |
There was a problem hiding this comment.
Not sure what this optimization refers to. Does it mean optimizing what ui5-tooling-modules does? Maybe this can be removed from this chapter?
There was a problem hiding this comment.
Couple of things:
- Extraction of shared packages into standalone packages
- This dedublication will lead to smaller parent packages, less size being downloaded, browser caching
There was a problem hiding this comment.
I have updated the document
rfcs/0020-npm-package-integration.md
Outdated
|
|
||
| - **UI5 Application Source** — The starting point is the developer's application code. Controllers reference NPM packages via AMD dependencies (e.g., `sap.ui.define(['thirdparty/chart.js', ...])`), and XML views reference them via xmlns declarations (e.g., `xmlns:webc="thirdparty.@ui5.webcomponents"`). These `thirdparty/*` references are the convention that triggers the NPM integration pipeline. | ||
|
|
||
| - **Phase 1: Integrated Dependency Analysis** — The existing `JSModuleAnalyzer` and `XMLTemplateAnalyzer` are extended to detect `thirdparty/*` patterns during their regular AST traversal. `espree.parse` walks the AST looking for `thirdparty/*` in AMD dependency arrays and ESM import statements, while `XMLTemplateAnalyzer` scans for `xmlns:thirdparty.*` declarations. `ResourceCollector` aggregates all detected NPM package names across all analyzed resources. **Key architectural decision**: This piggybacks on the AST parsing that already occurs for UI5 dependency analysis — no separate scanning phase is needed, eliminating duplicate parsing overhead. Output: a deduplicated set of NPM package names (e.g., `['chart.js', 'react', 'react-dom', '@ui5/webcomponents']`). |
There was a problem hiding this comment.
This thirdparty/* naming convention is only introduced in this concept, right? ui5-tooling-modules seems to simply use the npm package name. Why do we need the prefix here?
Wouldn't this break TypeScript use cases where an import for react would give you the corresponding types but thirdparty/react wouldn't resolve them anymore?
There was a problem hiding this comment.
This is an interesting point and I fully agree with it, but yes, it's kind of "new" concept here.
Here are my toughts on this:
- If we have a direct package name being referenced directlty i.e.
react, this means that this package is not UI5 AMD wrapped and the code that follows knows how to handle the raw format. The package is taken directly fromnode_mdules/ - Our current situation (in UI5 repositories) is that all the packages/thirdparties that are (usually) UI5 AMD wrapped and stay in a corresponding
thirdparty/folder.
There was a problem hiding this comment.
I have refactored the document to try to explain my points clearly
|
|
||
| - **Phase 1: Integrated Dependency Analysis** — The existing `JSModuleAnalyzer` and `XMLTemplateAnalyzer` are extended to detect `thirdparty/*` patterns during their regular AST traversal. `espree.parse` walks the AST looking for `thirdparty/*` in AMD dependency arrays and ESM import statements, while `XMLTemplateAnalyzer` scans for `xmlns:thirdparty.*` declarations. `ResourceCollector` aggregates all detected NPM package names across all analyzed resources. **Key architectural decision**: This piggybacks on the AST parsing that already occurs for UI5 dependency analysis — no separate scanning phase is needed, eliminating duplicate parsing overhead. Output: a deduplicated set of NPM package names (e.g., `['chart.js', 'react', 'react-dom', '@ui5/webcomponents']`). | ||
|
|
||
| - **Transitive Dependency Consolidation** — Before Rollup runs, each scanned package's full transitive dependency tree is enumerated by walking `package.json` `dependencies` fields recursively. Any transitive dependency appearing in two or more trees must be externalized — bundled as its own standalone AMD module rather than being inlined into each consumer's bundle. This produces an externals map (which deps to exclude per package) and may add newly-discovered shared packages to the bundle set (see [Section 3.8.3](#383-externals-discovery-via-transitive-dependency-consolidation)). |
There was a problem hiding this comment.
Keep in mind that @ui5/project already contains some functionality to traverse npm dependencies which could be reused here: https://github.com/UI5/cli/blob/main/packages/project/lib/graph/providers/NodePackageDependencies.js
There was a problem hiding this comment.
Thanks a lot!
I'll take a look at it. As you can see in the PoC, currently we have the "external" (transitive) dependencies being hardocded as I wanted to try the feasibility of the build and the expected output
|
|
||
| #### 3.4.2 The `thirdparty/*` Convention | ||
|
|
||
| The expected namespace convention for NPM packages in UI5 code: |
There was a problem hiding this comment.
See comment above, why are we introducing this convention?
There was a problem hiding this comment.
It would be a lot easier if we have a concept like this one.
Otherwise, we must do some kind of heuristics in order to identify external packages which might be fragile and hard to implement.
#1287 (comment)
|
|
||
| #### 3.7.3 Advanced Edge Cases | ||
|
|
||
| These edge cases are handled based on `ui5-tooling-modules` production experience: |
There was a problem hiding this comment.
This describes what ui5-tooling-modules provides. We should also document which of these we need to implement in UI5 CLI as well
There was a problem hiding this comment.
That's exactly the idea- to discuss what we can and want to deliver
|
|
||
| #### 3.8.1 Design Principle | ||
|
|
||
| **Core concept:** One NPM package = One AMD module file. |
There was a problem hiding this comment.
I'm confused by this. Won't rollup in some cases bundle multiple npm packages into one AMD module that is then imported in the app?
There was a problem hiding this comment.
Maybe, I need to be more clear here. It's more like:
one entry point (import/require/sap.ui.define) in UI5 source files === npm package
But this does not mean that the npm package is standalone. The build is handled by rollup and It can be bundled along with its transitive dependencies.
|
|
||
| The externals + paths mechanism solves this: `react-dom` and `react-colorful` declare `react` as external, and their AMD output references `thirdparty/react` instead of inlining React's code. | ||
|
|
||
| #### 3.8.3 Externals Discovery via Transitive Dependency Consolidation |
There was a problem hiding this comment.
Is my understanding correct that this will also define the entry points for rollup?
There was a problem hiding this comment.
Yes!
This is the idea of deduplication of (transitive dependencies) packages that will be resued by other packages.
Those transitive dependencies won't be used directly in UI5's app source code, but by thirdparties, dependending on them
|
|
||
| #### 3.8.4 Build Order Optimization | ||
|
|
||
| At **build time**, packages must be bundled in topological order (dependencies first) so that externals are resolved correctly: |
There was a problem hiding this comment.
Is this something we need to be aware of? I would think this is handled within rollup.
There was a problem hiding this comment.
Unfortunatelly, not!
As far as I got this, we must first build the "clean" dependencies, so that they can be reused (as imports) for the rest of bundling. Otherwise, rollup might:
- not discover them correctly
- bundle everything
|
|
||
| #### 3.10.1 Motivation | ||
|
|
||
| NPM packages sometimes contain bugs, security vulnerabilities, or incompatibilities that the upstream maintainer has not yet fixed — or cannot fix in a timely manner. In these cases, the consuming project must apply a targeted fix (patch) to the package source *before* Rollup bundles it. Without a patching mechanism, the only alternatives are forking the package or waiting for an upstream release. |
There was a problem hiding this comment.
I think we should discuss whether patching should be part of the initial release of this feature. I'm still uncomfortable adding such functionality while most other web frameworks do not provide something similar.
There was a problem hiding this comment.
We must definitelly discuss this! It's just an idea as we have a lot of packges being patched currently.
However, when we have the possibility to update and monitor packages quickly via package.json, the idea of patching might become redundant.
WebComponents currently do that, for example: https://github.com/UI5/webcomponents/tree/main/.yarn/patches
They rely on yarn, though!
3 participants
JIRA: CPOUI5FOUNDATION-1196
JIRA: CPOUI5FOUNDATION-979
Implementations, researches and PoCs are based on the input from ui5-tooling-modules
RFC: https://github.com/UI5/cli/blob/d7d7c1e4460a28815597a1df543eda0cfa83464a/rfcs/0020-npm-package-integration.md
Sample app build & serve:
internal/npm-integration-poc/consolidated-app/Simple rollup tests and builds:
internal/npm-integration-poc/scenarios/Shared code (
utils):internal/npm-integration-poc/lib/