-
Notifications
You must be signed in to change notification settings - Fork 83
feat: Disable generation of library manifest #1533
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
base: main
Are you sure you want to change the base?
Changes from all commits
7169840
5e71d46
105bb67
f00c99b
f289522
e9d4b9c
8cc5857
d0e10cc
7cc5d60
5c192ff
c02d748
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -43,6 +43,23 @@ UI5 CLI 5.x introduces **Specification Version 5.0**, which enables the new Comp | |
|
|
||
| Projects using older **Specification Versions** are expected to be **fully compatible with UI5 CLI v5**. | ||
|
|
||
| ## generateLibraryManifest Task No Longer Executed | ||
|
|
||
| ::: info Specification Version 5.0 only | ||
| This change only applies to library projects that upgrade their `specVersion` to `5.0` in `ui5.yaml`. Projects on **Specification Version 4.0 and lower are not affected**. **SAPUI5 distribution libraries (framework projects) are not affected**. | ||
| ::: | ||
|
|
||
| With **Specification Version 5.0**, the [`generateLibraryManifest`](../api/module-@ui5_builder_tasks_generateLibraryManifest) build task is no longer executed for non-framework library projects. Libraries must provide a `manifest.json` directly in their source directory. | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If you take
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't want to judge on developers' experience and knowledge. It's clear now that they need to provide a IMO, we must not mix/couple our documentation with UI5 runtime information. It must be clear from the UI5 documentation where certain files must be positioned and all the edge cases that might come out of that. |
||
|
|
||
| **Action required** when upgrading a library project to Specification Version 5.0: | ||
|
|
||
| - Ensure your library has a `manifest.json` in its source directory. A previously auto-generated one is fully compatible and can be reused as-is. | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would rather have described this more literally, by telling to run a build before changing the specVersion and copy over the
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't feel this is the correct way to describe this. For me, it feels like a workaround over the old |
||
| - If no `manifest.json` is present, any UI5 CLI command that resolves the project graph (such as `ui5 build`, `ui5 serve`, or `ui5 tree`) will fail with a descriptive error and a link to this migration guide. | ||
|
|
||
| ::: tip | ||
| To see which standard tasks are executed for each project type, check out the [Standard Tasks](../pages/Builder#standard-tasks) table in the UI5 Builder page. | ||
| ::: | ||
|
|
||
| ## Build Cache | ||
|
|
||
| UI5 CLI v5 introduces **builds with caching** for both the `ui5 build` and `ui5 serve` commands. This fundamental architectural change significantly improves build performance by reusing cached results from previous builds. It also simplifies development with the server by making most custom middleware obsolete. | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -194,6 +194,23 @@ class Library extends ComponentProject { | |
| async _parseConfiguration(config, buildManifest) { | ||
| await super._parseConfiguration(config, buildManifest); | ||
|
|
||
| // For Specification Version 5.0+, non-framework libraries must provide a manifest.json | ||
| if (this.getSpecVersion().gte("5.0") && !this.isFrameworkProject()) { | ||
| try { | ||
| await this._getManifest(); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Here the check is done before the
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There's a manifest check in the Yes, Given that, my preference would be to keep this solution as clearer and more maintainable
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That is not what I meant. Yes, If an application project is already built, and therefore But now if a library project is already built, the existence of a
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, and now, after this task, the |
||
| } catch (err) { | ||
| throw new Error( | ||
| `Could not find required manifest.json for library project ${this.getName()}: ` + | ||
| `${err.message}\n\n` + | ||
| `Library projects using Specification Version 5.0 or higher must provide ` + | ||
| `a manifest.json directly in their source directory.\n` + | ||
| `For migration details, please refer to:\n` + | ||
| `https://ui5.github.io/cli/updates/migrate-v5/#generatelibrarymanifest-task-no-longer-executed`, { | ||
| cause: err | ||
| }); | ||
| } | ||
| } | ||
|
|
||
| if (buildManifest) { | ||
| this._namespace = buildManifest.namespace; | ||
| return; | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This section should be reviewed again by UA, but my thoughts would be:
Does the title still explain the changes well enough? It is not wrong, but something that describes the new expectations with specVersion 5, i.e. "manifest.json is mandatory" might be easer to understand for consumers, who might not even have heard of the task name before.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From my perspective, it states clear what we have done and what action needs to be taken- we have disabled {some task name} and
manifest.jsonis now mandatory when building a library. Further more, 7cc5d60 enforces develoeprs to provide amanifest.json