Skip to content

Conversation

alan-agius4
Copy link
Contributor

Commit a0b2d36#diff-3975e0ee5aa3e06ecbcd76f5fa5134612f7fd2e6802ca7d370973bd410aab55cR25-R31 changed the serialization phase logic so that when the state is empty the script tag is not added to the document. As a side effect, this caused the toJson not called which caused the onSerialize callbacks also not to be called. These callbacks are used to provide the value for a key when toJson is called. Example: ngrx/platform#101 (comment)

Closes #47172

@pullapprove pullapprove bot requested a review from alxhub October 27, 2022 08:13
@alan-agius4 alan-agius4 added the area: server Issues related to server-side rendering label Oct 27, 2022
@ngbot ngbot bot added this to the Backlog milestone Oct 27, 2022
@alan-agius4 alan-agius4 added the target: patch This PR is targeted for the next patch release label Oct 27, 2022
@alan-agius4 alan-agius4 requested review from AndrewKushnir and removed request for alxhub October 27, 2022 08:17
Commit angular@a0b2d36#diff-3975e0ee5aa3e06ecbcd76f5fa5134612f7fd2e6802ca7d370973bd410aab55cR25-R31 changed the serialization phase logic so
that when the state is empty the script tag is not added to the document. As a side effect, this caused the `toJson` not called which caused the `onSerialize` callbacks also not to be called.
These callbacks are used to provide the value for a key when `toJson` is called. Example: ngrx/platform#101 (comment)

Closes angular#47172
@alan-agius4 alan-agius4 added the action: review The PR is still awaiting reviews from at least one requested reviewer label Oct 27, 2022
@alan-agius4 alan-agius4 changed the title fix(server): call onSerialize when state is empty fix(platform-server): call onSerialize when state is empty Oct 27, 2022
Copy link
Contributor

@AndrewKushnir AndrewKushnir left a comment

Choose a reason for hiding this comment

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

👍

@AndrewKushnir AndrewKushnir removed the action: review The PR is still awaiting reviews from at least one requested reviewer label Oct 27, 2022
@alan-agius4 alan-agius4 added the action: merge The PR is ready for merge by the caretaker label Oct 27, 2022
@alxhub
Copy link
Member

alxhub commented Oct 28, 2022

This PR was merged into the repository by commit 790ee17.

alxhub pushed a commit that referenced this pull request Oct 28, 2022
Commit a0b2d36#diff-3975e0ee5aa3e06ecbcd76f5fa5134612f7fd2e6802ca7d370973bd410aab55cR25-R31 changed the serialization phase logic so
that when the state is empty the script tag is not added to the document. As a side effect, this caused the `toJson` not called which caused the `onSerialize` callbacks also not to be called.
These callbacks are used to provide the value for a key when `toJson` is called. Example: ngrx/platform#101 (comment)

Closes #47172

PR Close #47888
@alxhub alxhub closed this in 790ee17 Oct 28, 2022
alxhub pushed a commit that referenced this pull request Oct 28, 2022
Commit a0b2d36#diff-3975e0ee5aa3e06ecbcd76f5fa5134612f7fd2e6802ca7d370973bd410aab55cR25-R31 changed the serialization phase logic so
that when the state is empty the script tag is not added to the document. As a side effect, this caused the `toJson` not called which caused the `onSerialize` callbacks also not to be called.
These callbacks are used to provide the value for a key when `toJson` is called. Example: ngrx/platform#101 (comment)

Closes #47172

PR Close #47888
@alan-agius4 alan-agius4 deleted the on-serial-cb branch October 28, 2022 09:57
crapStone pushed a commit to Calciumdibromid/CaBr2 that referenced this pull request Nov 4, 2022
This PR contains the following updates:

| Package | Type | Update | Change |
|---|---|---|---|
| [@angular/animations](https://github.com/angular/angular) | dependencies | patch | [`14.2.8` -> `14.2.9`](https://renovatebot.com/diffs/npm/@angular%2fanimations/14.2.8/14.2.9) |
| [@angular/common](https://github.com/angular/angular) | dependencies | patch | [`14.2.8` -> `14.2.9`](https://renovatebot.com/diffs/npm/@angular%2fcommon/14.2.8/14.2.9) |
| [@angular/compiler](https://github.com/angular/angular) | dependencies | patch | [`14.2.8` -> `14.2.9`](https://renovatebot.com/diffs/npm/@angular%2fcompiler/14.2.8/14.2.9) |
| [@angular/compiler-cli](https://github.com/angular/angular/tree/main/packages/compiler-cli) ([source](https://github.com/angular/angular)) | devDependencies | patch | [`14.2.8` -> `14.2.9`](https://renovatebot.com/diffs/npm/@angular%2fcompiler-cli/14.2.8/14.2.9) |
| [@angular/core](https://github.com/angular/angular) | dependencies | patch | [`14.2.8` -> `14.2.9`](https://renovatebot.com/diffs/npm/@angular%2fcore/14.2.8/14.2.9) |
| [@angular/forms](https://github.com/angular/angular) | dependencies | patch | [`14.2.8` -> `14.2.9`](https://renovatebot.com/diffs/npm/@angular%2fforms/14.2.8/14.2.9) |
| [@angular/platform-browser](https://github.com/angular/angular) | dependencies | patch | [`14.2.8` -> `14.2.9`](https://renovatebot.com/diffs/npm/@angular%2fplatform-browser/14.2.8/14.2.9) |
| [@angular/platform-browser-dynamic](https://github.com/angular/angular) | dependencies | patch | [`14.2.8` -> `14.2.9`](https://renovatebot.com/diffs/npm/@angular%2fplatform-browser-dynamic/14.2.8/14.2.9) |

---

### Release Notes

<details>
<summary>angular/angular</summary>

### [`v14.2.9`](https://github.com/angular/angular/blob/HEAD/CHANGELOG.md#&#8203;1429-2022-11-03)

[Compare Source](angular/angular@14.2.8...14.2.9)

##### platform-browser

| Commit | Type | Description |
| -- | -- | -- |
| [92d28bdd99](angular/angular@92d28bd) | perf | resolve memory leak when using animations with shadow DOM ([#&#8203;47903](angular/angular#47903)) |

##### platform-server

| Commit | Type | Description |
| -- | -- | -- |
| [d2d9bbf5ce](angular/angular@d2d9bbf) | fix | call `onSerialize` when state is empty ([#&#8203;47888](angular/angular#47888)) |

#### Special Thanks

Alan Agius, Kristiyan Kostadinov, Virginia Dooley and mgechev

<!-- CHANGELOG SPLIT MARKER -->

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about these updates again.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box

---

This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).
<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNC4xMy4yIiwidXBkYXRlZEluVmVyIjoiMzQuMTcuMSJ9-->

Co-authored-by: cabr2-bot <cabr2.help@gmail.com>
Reviewed-on: https://codeberg.org/Calciumdibromid/CaBr2/pulls/1626
Reviewed-by: Epsilon_02 <epsilon_02@noreply.codeberg.org>
Co-authored-by: Calciumdibromid Bot <cabr2_bot@noreply.codeberg.org>
Co-committed-by: Calciumdibromid Bot <cabr2_bot@noreply.codeberg.org>
@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 Nov 28, 2022
trekladyone pushed a commit to trekladyone/angular that referenced this pull request Feb 1, 2023
…#47888)

Commit angular@a0b2d36#diff-3975e0ee5aa3e06ecbcd76f5fa5134612f7fd2e6802ca7d370973bd410aab55cR25-R31 changed the serialization phase logic so
that when the state is empty the script tag is not added to the document. As a side effect, this caused the `toJson` not called which caused the `onSerialize` callbacks also not to be called.
These callbacks are used to provide the value for a key when `toJson` is called. Example: ngrx/platform#101 (comment)

Closes angular#47172

PR Close angular#47888
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: 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.

Check for TransferState.isEmpty prevents serialization
3 participants