8000 fix(platform-server): less aggressive ngServerMode cleanup by jkrems · Pull Request #61106 · angular/angular · GitHub
[go: up one dir, main page]

Skip to content

fix(platform-server): less aggressive ngServerMode cleanup #61106

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 sta 8000 tement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from

Conversation

jkrems
Copy link
Contributor
@jkrems jkrems commented May 3, 2025

Other code may depend on ngServerMode and it might have been set globally / via a bundler. Forcing it to undefined in those situations can lead to hard debug issues where the only symptom is that "suddenly" browser-specific code paths run on the server and (obviously) break.


Note: This is the less aggressive version that still forces ngServerMode to undefined if that's how we found it. I think it would also be reasonable to remove this cleanup all together but I tried to err on the side of caution.

@pullapprove pullapprove bot requested a review from kirjs May 3, 2025 02:21
@angular-robot angular-robot bot added the area: server Issues related to server-side rendering label May 3, 2025
@ngbot ngbot bot added this to the Backlog milestone May 3, 2025
@jkrems jkrems added target: patch This PR is targeted for the next patch release action: review The PR is still awaiting reviews from at least one requested reviewer labels May 3, 2025
Other code may depend on `ngServerMode` and it might have been set
globally / via a bundler. Forcing it to `undefined` in those situations
can lead to hard debug issues where the only symptom is that "suddenly"
browser-specific code paths run on the server and (obviously) break.
@jkrems jkrems force-pushed the jk-cond-server-mode-cleanup branch from 12bca02 to 4e69312 Compare May 3, 2025 23:31
@AndrewKushnir AndrewKushnir requested review from alan-agius4 and removed request for kirjs May 6, 2025 04:08
@pullapprove pullapprove bot requested a review from kirjs May 6, 2025 04:08
Copy link
Contributor
@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 requested review from thePunderWoman and removed request for kirjs May 6, 2025 05:22
Copy link
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

@alan-agius4 alan-agius4 added action: merge The PR is ready for merge by the caretaker and removed action: review The PR is still awaiting reviews from at least one requested reviewer labels May 6, 2025
AndrewKushnir pushed a commit that referenced this pull request May 6, 2025
Other code may depend on `ngServerMode` and it might have been set
globally / via a bundler. Forcing it to `undefined` in those situations
can lead to hard debug issues where the only symptom is that "suddenly"
browser-specific code paths run on the server and (obviously) break.

PR Close #61106
@AndrewKushnir
Copy link
Contributor

This PR was merged into the repository by commit 06d6da3.

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

AndrewKushnir pushed a commit that referenced this pull request May 6, 2025
Other code may depend on `ngServerMode` and it might have been set
globally / via a bundler. Forcing it to `undefined` in those situations
can lead to hard debug issues where the only symptom is that "suddenly"
browser-specific code paths run on the server and (obviously) break.

PR Close #61106
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
action: merge The PR is ready for merge by the caretaker area: server Issues related to server-side rendering target: patch This PR is targeted for the next patch release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants
0