Skip to content

fix(data): make mergeServerUpserts change state immutably (#2374)#2389

Merged
brandonroberts merged 1 commit into
ngrx:masterfrom
AdditionAddict:pr/fix_merge_server_upserts
Feb 18, 2020
Merged

fix(data): make mergeServerUpserts change state immutably (#2374)#2389
brandonroberts merged 1 commit into
ngrx:masterfrom
AdditionAddict:pr/fix_merge_server_upserts

Conversation

@AdditionAddict

Copy link
Copy Markdown
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?

Runtime error thrown if performing a QUERY action (with the default preserve changes strategy) after an UPSERT action due to mutably changing state.

can't define property "originalValue": Object is not extensible

Closes #2374

What is the new behavior?

mergeServerUpserts which is used by QUERY actions changes state immutably without error.

Does this PR introduce a breaking change?

[ ] Yes
[ x ] No

Other information

I believe this also fixes #2307

A failing test that simulates runtime checks is the following (not included as this becomes immediately redundant):

  describe('#mergeQueryResults', () => {
    it('should change state immutably', () => {
      let {
        updatedHero,
        serverUpdatedHero,
        initialCache,
      } = createInitialCacheForMerges();

      const test = function() {
        'use strict';
        // freeze part of original state
        Object.freeze(initialCache.Hero.changeState[updatedHero.id]!);

        const collection = tracker.mergeQueryResults(
          [serverUpdatedHero],
          initialCache.Hero
        );

        // check state does not hold copy of original state -  will throw error if this is the case
        collection.changeState[updatedHero.id]!.originalValue = updatedHero;
      };
      expect(test).not.toThrow();
    });

@ngrxbot

ngrxbot commented Feb 16, 2020

Copy link
Copy Markdown
Collaborator

Preview docs changes for 2b127aa at https://previews.ngrx.io/pr2389-2b127aa/

@brandonroberts brandonroberts left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

@brandonroberts brandonroberts merged commit b3a49c1 into ngrx:master Feb 18, 2020
@AdditionAddict AdditionAddict deleted the pr/fix_merge_server_upserts branch February 18, 2020 14:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

4 participants