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

feat: Entity fully denormalizes & Entity -> FlatEntity for non-nested #328

Merged
merged 2 commits into from
May 4, 2020

Conversation

ntucker
Copy link
Collaborator

@ntucker ntucker commented Apr 29, 2020

Fixes #301, #277, #165.

Motivation

Lacking full denormalization is a regression from recent consolidation. This allows for that.

Solution

  • Entity now fully supported nested denormalization - including recursion.
    • Use this as basis for tests now so they have similar results as before.
    • This does not guarantee referential equality every denorm call. This will need to be solved to eliminate FlatEntity. (this is the old schema.Entity behavior)
  • FlatEntity replicated previous Entity behavior, which is what rest-hooks uses to build Resource
  • Fixed up immutablejs usage to still result in Entity construction.
  • All ids in normalizr tests are now strings - as that is the only allowed pk type.

To maintain backwards compatibility, exports from 'rest-hooks' package itself will be NestedEntity and Entity - that have previous behavior. New projects should import Entity and FlatEntity from @rest-hooks/normalizr directly.

Open questions

The next step is to make referential equality last more than one denorm call.

Entity, and it's schema members will need to be the keys mapping into a fully denormalized object. These should be freed from memory as soon as all other references to the fully denormalized object are lost.

{
  this: this,
  draftedBy: USERA,
  publishedBy USERB,
} -> fullyDenorm(members)

@ntucker ntucker force-pushed the full-denormalize branch 2 times, most recently from f239289 to 0e25690 Compare May 2, 2020 22:06
@github-actions
Copy link
Contributor

github-actions bot commented May 2, 2020

Size Change: +90 B (0%)

Total Size: 26.9 kB

Filename Size Change
packages/rest-hooks/dist/index.cjs.js 15.6 kB +81 B (0%)
packages/rest-hooks/dist/index.umd.min.js 6.94 kB +9 B (0%)
ℹ️ View Unchanged
Filename Size Change
packages/legacy/dist/index.cjs.js 433 B 0 B
packages/legacy/dist/index.umd.min.js 376 B 0 B
packages/test/dist/index.cjs.js 1.86 kB 0 B
packages/use-enhanced-reducer/dist/index.cjs.js 1.08 kB 0 B
packages/use-enhanced-reducer/dist/index.umd.min.js 593 B 0 B

compressed-size-action

@ntucker ntucker changed the title feat: full denormalize feat: NestingEntity with full denormalization May 3, 2020
@ntucker ntucker force-pushed the full-denormalize branch 3 times, most recently from c631018 to 03a0268 Compare May 4, 2020 15:47
@ntucker ntucker changed the title feat: NestingEntity with full denormalization feat: Entity fully denormalizes & Entity -> FlatEntity for non-nested May 4, 2020
@@ -233,17 +233,16 @@ describe(`${Entity.name} normalization`, () => {
});
});

/* TODO: This isn't supported quite yet
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

note we re-enable this entire block. This has a lot of interested recursively nested denormalization cases.

@@ -32,14 +26,19 @@ import useSelectionUnstable from './react-integration/hooks/useSelection';
import hasUsableData from './react-integration/hooks/hasUsableData';
import { StateContext, DispatchContext } from './react-integration/context';

export {
Entity as NestedEntity,
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

remapping so there's no actual breaking changes for 'rest-hooks' itself (for now)

Copy link

@nickcherry nickcherry left a comment

Choose a reason for hiding this comment

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

looks good to me! 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[docs] Immutability
2 participants