Skip to content

Conversation

@leosvelperez
Copy link
Contributor

@leosvelperez leosvelperez commented May 5, 2023

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 index option of the browser-esbuild builder is defined to accept one of a string, an object, or a const "false". Note the const value it's not false but a string value. Because of that, there's no way to disable the index generation:

  • If we set the option to "false" we get an error because it matches both the string and the const:
     Error: Schema validation failed with the following errors:
     Data path "/index" must be object.
     Data path "/index" must match exactly one schema in oneOf.
    
  • If we set the option to false we get an error because it doesn't match any of the possible definitions:
     Error: Schema validation failed with the following errors:
     Data path "/index" must be string.
     Data path "/index" must be object.
     Data path "/index" must be equal to constant.
     Data path "/index" must match exactly one schema in oneOf.
    
  • If we run the build with ng build --no-index (as one of the goals of the change 0d4a40f) we get the same as the previous error:
    Error: Schema validation failed with the following errors:
    Data path "/index" must be string.
    Data path "/index" must be object.
    Data path "/index" must be equal to constant.
    Data path "/index" must match exactly one schema in oneOf.
    

Issue Number: N/A

What is the new behavior?

The index option of the browser-esbuild builder is defined to accept one of a string, and object or a const false (boolean). This allows users to set the option to false in their angular.json or to run the build with --no-index.

Does this PR introduce a breaking change?

  • Yes
  • No

Other information

Copy link
Collaborator

@alan-agius4 alan-agius4 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@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 labels May 5, 2023
@angular-robot angular-robot bot merged commit e15100d into angular:main May 8, 2023
@leosvelperez leosvelperez deleted the fix/esbuild-index-option-const-value branch May 8, 2023 13:12
@angular-automatic-lock-bot
Copy link

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 Jun 8, 2023
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