Skip to content

Conversation

jsaguet
Copy link
Contributor

@jsaguet jsaguet commented Jan 5, 2025

Fixes #29145

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?

_defineProperty is marked as pure and then tree shaken when targeting Safari < 15 or Chrome < 84 which breaks the application.

Issue Number: #29145

What is the new behavior?

_defineProperty is not marked as pure

Does this PR introduce a breaking change?

  • Yes
  • No

Other information

@jsaguet jsaguet changed the title fix: do not mark _defineProperty as pure in build output fix(@angular/build): do not mark _defineProperty as pure in build output Jan 5, 2025
@jsaguet jsaguet force-pushed the fix/impure-define-property branch from bac5d43 to d9cbfcb Compare January 5, 2025 19:52
@jsaguet jsaguet force-pushed the fix/impure-define-property branch from d9cbfcb to 3bdc9c3 Compare January 5, 2025 20:06
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, just one NIT

@alan-agius4 alan-agius4 added the target: patch This PR is targeted for the next patch release label Jan 6, 2025
@jsaguet jsaguet force-pushed the fix/impure-define-property branch from 3bdc9c3 to 46c1622 Compare January 6, 2025 09:30
@angular-robot angular-robot bot requested a review from alan-agius4 January 6, 2025 09:30
@jsaguet jsaguet changed the title fix(@angular/build): do not mark _defineProperty as pure in build output fix(@angular/build): do not mark Babel _defineProperty helper function as pure Jan 6, 2025
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, thanks for your contribution!

@alan-agius4 alan-agius4 added the action: merge The PR is ready for merge by the caretaker label Jan 6, 2025
@alan-agius4 alan-agius4 merged commit a561869 into angular:main Jan 6, 2025
31 checks passed
@alan-agius4
Copy link
Collaborator

The changes were merged into the following branches: main, 19.0.x

@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 Feb 6, 2025
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 area: @angular/build target: patch This PR is targeted for the next patch release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

App does not start when downleveling past chrome 84 or safari 15 since angular 19

2 participants