-
Notifications
You must be signed in to change notification settings - Fork 23
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
[FEATURE] Bundle require
section with async flag for specVersion: 4.0
#1042
Conversation
The build fails as for some reason Azure tries to install Node 20.15.0, which has some isseues with Buffers. Edit: Fixed with #1043 |
dcb3155
to
bb33e24
Compare
@@ -135,6 +135,7 @@ const log = getLogger("builder:processors:bundlers:moduleBundler"); | |||
* @param {string} [parameters.options.targetUi5CoreVersion] Optional semver compliant sap.ui.core project version, e.g '2.0.0'. | |||
This allows the bundler to make assumptions on available runtime APIs. | |||
Omit if the ultimate UI5 version at runtime is unknown or can't be determined. | |||
* @param {@ui5/project/build/helpers/TaskUtil|object} [parameters.taskUtil] TaskUtil |
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.
obsolete parameter? or was it already in use but undocumented?
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.
// If specVersion cannot be determined, the latest spec is taken into account. | ||
// This is a breaking change in specVersion: 4.0 | ||
if (section.mode === "require" && (!taskUtil || taskUtil.getProject().getSpecVersion().gte("4.0"))) { | ||
defaultValues.async = true; |
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.
Is there a test that verifies this behavior?
The bundle tooling already defaults to "async=true", so I doubt that this line has any effect. My expectation would be that the flag has to be actively set to false in the else case, as this is the value that differs from the new default behavior.
I tested this with a self-contained build and even with specVersion 3.0 I get an async require section, which is not expected.
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.
We should also document the "async" option in the respective JSDoc: https://github.com/SAP/ui5-builder/blob/main/lib/processors/bundlers/moduleBundler.js#L13
JIRA: CPOUI5FOUNDATION-835
This change completes #1033 with check for specVersion: 4.0 and enforcing default async flag for require section in
bundleDefinitions