Skip to content
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 statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(storage-plugin): only restore state if key matches addedStates #1746

Merged
merged 3 commits into from
Jun 21, 2021

Conversation

rhutchison
Copy link
Contributor

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

[x] 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?

This plugin is reacting to the InitState event as well as any UpdateState events.

When state is loaded lazily, it triggers an UpdateState event that passes along the states being added addedStates. Each time an UpdateState event is triggered, it causes the state to be restored for each key defined in storage-plugin. In cases where there are multiple lazily loaded state, this causes multiple setState actions to be triggered, and any subscriptions of that state, will fire.

Issue Number: N/A

What is the new behavior?

InitState will continue to restore state from storage engine for all defined keys.

UpdateState will only restore from storage engine if key matches addedStates.

Does this PR introduce a breaking change?

[ ] Yes
[x] No

Other information

@arturovt
Copy link
Member

arturovt commented May 7, 2021

The build is failing. Could you also add tests and update the changelog?

@rhutchison rhutchison force-pushed the fix/storage-plugin branch 3 times, most recently from 27dd65d to 6211929 Compare May 10, 2021 14:00
@splincode
Copy link
Member

Please, create unit test))

@splincode
Copy link
Member

@rhutchison any updates?

@markwhitfeld
Copy link
Member

@rhutchison, nice fix! Tests please... Maybe get one of your team to do them 😉 👀

@arturovt arturovt force-pushed the fix/storage-plugin branch 3 times, most recently from 6dd1971 to a828032 Compare June 18, 2021 05:09
@arturovt arturovt force-pushed the fix/storage-plugin branch 2 times, most recently from 39d800a to 2fdc68c Compare June 18, 2021 05:16
@arturovt
Copy link
Member

arturovt commented Jun 18, 2021

@markwhitfeld could you review?

@arturovt arturovt force-pushed the fix/storage-plugin branch from 2fdc68c to 84e906e Compare June 18, 2021 12:44
@markwhitfeld
Copy link
Member

@splincode Over to you for review ;-)

Copy link
Member

@splincode splincode left a comment

Choose a reason for hiding this comment

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

great job)

@arturovt arturovt merged commit 2f9542d into ngxs:master Jun 21, 2021
@arturovt arturovt deleted the fix/storage-plugin branch June 21, 2021 13:21
@markwhitfeld markwhitfeld added this to the v3.7.4 milestone Jun 10, 2022
crapStone pushed a commit to Calciumdibromid/CaBr2 that referenced this pull request Jun 17, 2022
This PR contains the following updates:

| Package | Type | Update | Change |
|---|---|---|---|
| [@ngxs/form-plugin](https://github.com/ngxs/store) | dependencies | patch | [`3.7.3` -> `3.7.4`](https://renovatebot.com/diffs/npm/@ngxs%2fform-plugin/3.7.3/3.7.4) |
| [@ngxs/storage-plugin](https://github.com/ngxs/store) | dependencies | patch | [`3.7.3` -> `3.7.4`](https://renovatebot.com/diffs/npm/@ngxs%2fstorage-plugin/3.7.3/3.7.4) |
| [@ngxs/store](https://github.com/ngxs/store) | dependencies | patch | [`3.7.3` -> `3.7.4`](https://renovatebot.com/diffs/npm/@ngxs%2fstore/3.7.3/3.7.4) |

---

### Release Notes

<details>
<summary>ngxs/store</summary>

### [`v3.7.4`](https://github.com/ngxs/store/blob/HEAD/CHANGELOG.md#&#8203;374-2022-06-09)

[Compare Source](ngxs/store@v3.7.3...v3.7.4)

-   Build: include support for Angular 14 [#&#8203;1850](ngxs/store#1850)
-   Fix: Do not re-use the global `Store` instance between different apps [#&#8203;1740](ngxs/store#1740) and [#&#8203;1804](ngxs/store#1804)
-   Fix: Handle mixed async scenarios for action handlers [#&#8203;1762](ngxs/store#1762)
-   Fix: An action with cancelUncompleted enabled should unsubscribe before the next action handler is called [#&#8203;1763](ngxs/store#1763)
-   Fix: Do not run `Promise.then` within synchronous tests when decorating factory [#&#8203;1753](ngxs/store#1753)
-   Fix: Provide `NoopNgxsExecutionStrategy` explicitly when the zone is nooped [#&#8203;1819](ngxs/store#1819)
-   Fix: Complete the state stream once the root view is removed [#&#8203;1830](ngxs/store#1830)
-   Fix: Be more explicit when checking if Angular is in test mode [#&#8203;1831](ngxs/store#1831), [#&#8203;1832](ngxs/store#1832)
-   Fix: Devtools Plugin - Do not connect to devtools when the plugin is disabled [#&#8203;1761](ngxs/store#1761)
-   Fix: Router Plugin - Cleanup subscriptions when the root view is destroyed [#&#8203;1754](ngxs/store#1754)
-   Fix: WebSocket Plugin - Cleanup subscriptions and close the connection when the root view is destroyed [#&#8203;1755](ngxs/store#1755)
-   Fix: Storage Plugin - Only restore state if key matches `addedStates` [#&#8203;1746](ngxs/store#1746)
-   Fix: Forms Plugin - Do not destructure primitive types [#&#8203;1845](ngxs/store#1845)
-   Performance: Tree-shake errors and warnings [#&#8203;1732](ngxs/store#1732)
-   Performance: Tree-shake `ConfigValidator`, `HostEnvironment` and `isAngularInTestMode` [#&#8203;1741](ngxs/store#1741)
-   Performance: Tree-shake `SelectFactory` [#&#8203;1744](ngxs/store#1744)
-   Performance: Tree-shake `deepFreeze` [#&#8203;1819](ngxs/store#1819)
-   Performance: Run change detection once for all selectors when asynchronous action has been completed [#&#8203;1828](ngxs/store#1828)
-   Performance: Router Plugin - Tree-shake `isAngularInTestMode()` [#&#8203;1738](ngxs/store#1738)
-   Performance: Tree-shake `isAngularInTestMode()` [#&#8203;1739](ngxs/store#1739)
-   Performance: Storage Plugin - Tree-shake `console.*` calls and expand error messages [#&#8203;1727](ngxs/store#1727)
-   CI: Bundlesize checks should run reliably [#&#8203;1812](ngxs/store#1812)

</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, click this checkbox.

---

This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).

Co-authored-by: cabr2-bot <[email protected]>
Reviewed-on: https://codeberg.org/Calciumdibromid/CaBr2/pulls/1401
Co-authored-by: Calciumdibromid Bot <[email protected]>
Co-committed-by: Calciumdibromid Bot <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants