Skip to content

refactor(@angular-devkit/build-angular): remove deployUrl from browser-esbuild builder-status-warnings#26625

Merged
alan-agius4 merged 1 commit intoangular:mainfrom
dominicbachmann:refactor/remove-deploy-url-from-browser-esbuild-builder-status-warnings
Dec 11, 2023
Merged

refactor(@angular-devkit/build-angular): remove deployUrl from browser-esbuild builder-status-warnings#26625
alan-agius4 merged 1 commit intoangular:mainfrom
dominicbachmann:refactor/remove-deploy-url-from-browser-esbuild-builder-status-warnings

Conversation

@dominicbachmann
Copy link
Copy Markdown
Contributor

PR Checklist

Please check to confirm your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Documentation content changes
  • Other... Please describe:

What is the current behavior?

The logBuilderStatusWarnings function of browser-esbuild builder checks if unsupportedOption is "deployUrl". However, deployUrl is not in the UNSUPPORTED_OPTIONS array.

Issue Number: N/A

What is the new behavior?

The logBuilderStatusWarnings function of browser-esbuild builder does not check if unsupportedOption is "deployUrl".

Does this PR introduce a breaking change?

  • Yes
  • No

Other information

@dominicbachmann
Copy link
Copy Markdown
Contributor Author

I also wondered if we could change this part

if (value === undefined || value === false) {
  continue;
}
if (Array.isArray(value) && value.length === 0) {
  continue;
}
if (typeof value === 'object' && Object.keys(value).length === 0) {
  continue;
}

to just

if (value === undefined) {
  continue;
}

because as a user I would still be interested if there is an entry in my angular.json which I could remove. So that e.g. "vendorChunk": false would still report to the user.

@alan-agius4
Copy link
Copy Markdown
Collaborator

The above mentioned check is still required. As when option is not provided by the users, it will have a default provided in the schema.

@alan-agius4 alan-agius4 added target: patch This PR is targeted for the next patch release action: merge The PR is ready for merge by the caretaker dependencies Pull requests that update a dependency file and removed dependencies Pull requests that update a dependency file labels Dec 11, 2023
@alan-agius4 alan-agius4 merged commit 52bf11f into angular:main Dec 11, 2023
@dominicbachmann dominicbachmann deleted the refactor/remove-deploy-url-from-browser-esbuild-builder-status-warnings branch December 11, 2023 13:02
@angular-automatic-lock-bot
Copy link
Copy Markdown

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Jan 11, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

action: merge The PR is ready for merge by the caretaker target: patch This PR is targeted for the next patch release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants