Skip to content

refactor(core): swap dev/prod error handling order in injector for tree-shaking#63354

Closed
arturovt wants to merge 1 commit intoangular:mainfrom
arturovt:refactor/core-inline-prependTokenToDependencyPath
Closed

refactor(core): swap dev/prod error handling order in injector for tree-shaking#63354
arturovt wants to merge 1 commit intoangular:mainfrom
arturovt:refactor/core-inline-prependTokenToDependencyPath

Conversation

@arturovt
Copy link
Copy Markdown
Contributor

@arturovt arturovt commented Aug 24, 2025

In production builds, ngDevMode is replaced with false, so the guard compiles to return;. However, bundlers like ESBuild still keep the remaining statements after the return as unreachable code instead of removing them. This leaves behind unnecessary dead code in the output.

Technically, the body is unreachable. But to prove that, the bundler must be 100% certain that:

  • return cannot be removed by some transform
  • there's no later transformation that changes control flow

As thus, it's always conservative.

This also allows dropping assertDefined, which was previously
referenced only inside prependTokenToDependencyPath. With the
function now fully inlined and dev-only, assertDefined is also
eliminated from production builds, further reducing bundle size.

@pullapprove pullapprove bot requested a review from thePunderWoman August 24, 2025 21:18
@angular-robot angular-robot bot added the area: core Issues related to the framework runtime label Aug 24, 2025
@ngbot ngbot bot added this to the Backlog milestone Aug 24, 2025
@JeanMeche
Copy link
Copy Markdown
Member

JeanMeche commented Aug 24, 2025

Do you have an esbuild issue reference for why this isn't removed by tree shaking ?

@arturovt
Copy link
Copy Markdown
Contributor Author

evanw/esbuild#4095

@JeanMeche
Copy link
Copy Markdown
Member

What if we moved prependTokenToDependencyPath into r3_injector (so it's not exported anymore). Does it get tree shaken away ?

@arturovt
Copy link
Copy Markdown
Contributor Author

Nope, I tried it :(

…ee-shaking

In production builds, `ngDevMode` is replaced with `false`, so the guard compiles to `return;`. However, bundlers like ESBuild still keep the remaining statements after the return as unreachable code instead of removing them. This leaves behind unnecessary dead code in the output.

Technically, the body is unreachable. But to prove that, the bundler must be 100% certain that:

- `return` cannot be removed by some transform
- there's no later transformation that changes control flow

As thus, it's always conservative.

This also allows dropping `assertDefined`, which was previously
referenced only inside `prependTokenToDependencyPath`. With the
function now fully inlined and dev-only, `assertDefined` is also
eliminated from production builds, further reducing bundle size.
@arturovt arturovt force-pushed the refactor/core-inline-prependTokenToDependencyPath branch from c0792d8 to e8c0d5a Compare September 16, 2025 08:44
@arturovt arturovt changed the title refactor(core): inline prependTokenToDependencyPath to enable tree-shaking @arturovt refactor(core): swap dev/prod error handling order in injector for tree-shaking Sep 16, 2025
@arturovt
Copy link
Copy Markdown
Contributor Author

@JeanMeche I re-worked it and removed an early return, should be looking better now

@arturovt arturovt changed the title @arturovt refactor(core): swap dev/prod error handling order in injector for tree-shaking refactor(core): swap dev/prod error handling order in injector for tree-shaking Sep 16, 2025
Copy link
Copy Markdown
Contributor

@thePunderWoman thePunderWoman left a comment

Choose a reason for hiding this comment

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

LGTM

@thePunderWoman thePunderWoman added action: merge The PR is ready for merge by the caretaker target: patch This PR is targeted for the next patch release labels Sep 16, 2025
thePunderWoman pushed a commit that referenced this pull request Sep 16, 2025
…ee-shaking (#63354)

In production builds, `ngDevMode` is replaced with `false`, so the guard compiles to `return;`. However, bundlers like ESBuild still keep the remaining statements after the return as unreachable code instead of removing them. This leaves behind unnecessary dead code in the output.

Technically, the body is unreachable. But to prove that, the bundler must be 100% certain that:

- `return` cannot be removed by some transform
- there's no later transformation that changes control flow

As thus, it's always conservative.

This also allows dropping `assertDefined`, which was previously
referenced only inside `prependTokenToDependencyPath`. With the
function now fully inlined and dev-only, `assertDefined` is also
eliminated from production builds, further reducing bundle size.

PR Close #63354
@thePunderWoman
Copy link
Copy Markdown
Contributor

This PR was merged into the repository. The changes were merged into the following branches:

@arturovt arturovt deleted the refactor/core-inline-prependTokenToDependencyPath branch September 16, 2025 15:55
wildcardalice pushed a commit to wildcardalice/angular that referenced this pull request Sep 18, 2025
…ee-shaking (angular#63354)

In production builds, `ngDevMode` is replaced with `false`, so the guard compiles to `return;`. However, bundlers like ESBuild still keep the remaining statements after the return as unreachable code instead of removing them. This leaves behind unnecessary dead code in the output.

Technically, the body is unreachable. But to prove that, the bundler must be 100% certain that:

- `return` cannot be removed by some transform
- there's no later transformation that changes control flow

As thus, it's always conservative.

This also allows dropping `assertDefined`, which was previously
referenced only inside `prependTokenToDependencyPath`. With the
function now fully inlined and dev-only, `assertDefined` is also
eliminated from production builds, further reducing bundle size.

PR Close angular#63354
@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 Oct 17, 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: core Issues related to the framework runtime target: patch This PR is targeted for the next patch release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants