8000 Fix nested cache merging for projections by hjdivad · Pull Request #1733 · ember-m3/ember-m3 · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Fix nested cache merging for projections #1733

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Feb 17, 2023
Merged

Fix nested cache merging for projections #1733

merged 1 commit into from
Feb 17, 2023

Conversation

hjdivad
Copy link
Collaborator
@hjdivad hjdivad commented Feb 16, 2023

Nested models would not previously be merged under the following
conditions:

  1. the nested models were projections of base models
  2. the userland computeBaseModelName hook returned an undasherized
    model name for nested models

This was due to a missing call to dasherize, to keep names normalized
for Ember Data.

@hjdivad hjdivad added the bug ch:bugfix label Feb 16, 2023
@hjdivad hjdivad force-pushed the hjdivad/merge-nested-fix branch from 3e8877c to 95520c2 Compare February 16, 2023 21:06
@hjdivad hjdivad enabled auto-merge February 16, 2023 21:12
@github-actions
Copy link
github-actions bot commented Feb 16, 2023 •
edited
Loading

Performance Report for 27f3ade

Scenario - materializing-lots: ☑️ Performance is stable

☑️ duration
phase no difference [-59ms to 14ms]
☑️ Phase [navigationStart] => [start-loading]
phase no difference [-11ms to 5ms]
☑️ Phase [start-loading] => [pushed-payload]
phase no difference [-44ms to 6ms]
☑️ Phase [pushed-payload] => [end-loading]
phase no difference [-11ms to 26ms]
✅ Phase [end-loading] => [Test End]
phase estimated improvement -1ms [-1ms to 0ms] OR -2.5% [-4.79% to -0.34%]

Scenario - materializing-small: ☑️ Performance is stable

☑️ duration
phase no difference [-10ms to 4ms]
☑️ Phase [navigationStart] => [start-loading]
phase no difference [-9ms to 3ms]
☑️ Phase [start-loading] => [pushed-payload]
phase no difference [-1ms to 1ms]
☑️ Phase [pushed-payload] => [end-loading]
phase no difference [-1ms to 1ms]
☑️ Phase [end-loading] => [Test End]
phase no difference [-1ms to 0ms]

Scenario - rendering: ☑️ Performance is stable

☑️ duration
phase no difference [-11ms to 8ms]
☑️ Phase [navigationStart] => [start-loading]
phase no difference [-1ms to 10ms]
✅ Phase [start-loading] => [pushed-payload]
phase estimated improvement -1ms [-2ms to 0ms] OR -2.75% [-4.92% to -0.5%]
☑️ Phase [pushed-payload] => [end-loading]
phase no difference [0ms to 0ms]
✅ Phase [end-loading] => [Test End]
phase estimated improvement -6ms [-12ms to -1ms] OR -1.07% [-1.95% to -0.2%]

@hjdivad hjdivad force-pushed the hjdivad/merge-nested-fix branch 2 times, most recently from 967bc2f to 27f3ade Compare February 16, 2023 22:57
@hjdivad
Fix cache merging
2ba0f89 
Nested models would not previously be merged under the following
conditions:
  1. the nested models were projections of base models
  2. the userland `computeBaseModelName` hook returned an undasherized
     model name for nested models

This was due to a missing call to dasherize, to keep names normalized
for Ember Data.
@hjdivad hjdivad force-pushed the hjdivad/merge-nested-fix branch from 27f3ade to 2ba0f89 Compare February 17, 2023 17:40
@hjdivad hjdivad merged commit 4a55742 into master Feb 17, 2023
@hjdivad hjdivad deleted the hjdivad/merge-nested-fix branch February 17, 2023 17:51
@hjdivad hjdivad changed the title Fix cache merging Fix nested cache merging for projections Feb 17, 2023
larry-x-yu pushed a commit to larry-x-yu/ember-m3 that referenced this pull request Apr 6, 2023
Revert "Merge pull request ember-m3#1733 from hjdivad/hjdivad/merge-n…
70EB f4da17b 
…ested-fix"

This reverts commit 4a55742, reversing
changes made to 95040bb.
This was referenced Apr 6, 2023
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
bug ch:bugfix
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
1 participant
@hjdivad
0