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: backgroundReloads should not affect recordIsLoaded (#8125) #8128

Merged
merged 2 commits into from
Aug 18, 2022

Conversation

runspired
Copy link
Contributor

Currently doesn't appear to be a regression here, but probably a good test to have around anyway. The regression might be in the timing of the promise-record setup, if so we'll need more info to track it down. Another hypothesis is there is an unloadRecord call involved which previously the findRecord call was "cancelling". Now the unloadRecord call would succeed and the data be cleared and refetch required.

@runspired runspired added 🎯 canary PR is targeting canary (default) 🏷️ test This PR primarily adds tests for a feature labels Aug 16, 2022
@github-actions
Copy link

github-actions bot commented Aug 17, 2022

Asset Size Report for 5a75092

Modern Builds

✅ EmberData shrank by -203.0 B (-54.0 B compressed)

If any packages had changed sizes they would be listed here.

Changeset


EmberData +168.65 KB -203.0 B (+35.27 KB -54.0 B compressed)
    @ember-data/store +46.71 KB -182.0 B (+9.77 KB -48.41 B compressed)
        @ember-data/store/-private +45.71 KB -182.0 B (+9.56 KB -48.41 B compressed)
    @ember-data/model +45.36 KB -21.0 B (+9.48 KB -5.59 B compressed)
        @ember-data/model/-private +44.88 KB -21.0 B (+9.39 KB -5.59 B compressed)

Full Asset Analysis (Modern)

Asset Size Report
=================


Library: EmberData
┌────────────┬─────────────┐
│  (index)   │   Values    │
├────────────┼─────────────┤
│   bytes    │ '168.65 KB' │
│ compressed │ '35.27 KB'  │
│  packages  │      8      │
│  modules   │     44      │
└────────────┴─────────────┘

Package: @ember-data/store
┌──────────────┬────────────┐
│   (index)    │   Values   │
├──────────────┼────────────┤
│    bytes     │ '46.71 KB' │
│  compressed  │ '9.77 KB'  │
│ % Of Library │   '27.7'   │
└──────────────┴────────────┘
	Module                                            | Bytes     | Compressed | % of Package  | % Of Library
	-----------------------------------------------------------------------------------------------------
	@ember-data/store/-private                        | 45.71 KB  | 9.56 KB    | 97.9          | 27.1
	@ember-data/store/index                           | 1022.00 B | 213.71 B   | 2.1           | 0.6

Package: @ember-data/model
┌──────────────┬────────────┐
│   (index)    │   Values   │
├──────────────┼────────────┤
│    bytes     │ '45.36 KB' │
│  compressed  │ '9.48 KB'  │
│ % Of Library │   '26.9'   │
└──────────────┴────────────┘
	Module                                            | Bytes     | Compressed | % of Package  | % Of Library
	-----------------------------------------------------------------------------------------------------
	@ember-data/model/-private                        | 44.88 KB  | 9.39 KB    | 99.0          | 26.6
	@ember-data/model/index                           | 486.00 B  | 101.63 B   | 1.0           | 0.3

Package: @ember-data/record-data
┌──────────────┬────────────┐
│   (index)    │   Values   │
├──────────────┼────────────┤
│    bytes     │ '22.25 KB' │
│  compressed  │ '4.65 KB'  │
│ % Of Library │   '13.2'   │
└──────────────┴────────────┘
	Module                                            | Bytes     | Compressed | % of Package  | % Of Library
	-----------------------------------------------------------------------------------------------------
	@ember-data/record-data/-private                  | 22.25 KB  | 4.65 KB    | 100.0         | 13.2

Package: @ember-data/serializer
┌──────────────┬────────────┐
│   (index)    │   Values   │
├──────────────┼────────────┤
│    bytes     │ '20.42 KB' │
│  compressed  │ '4.27 KB'  │
│ % Of Library │   '12.1'   │
└──────────────┴────────────┘
	Module                                            | Bytes     | Compressed | % of Package  | % Of Library
	-----------------------------------------------------------------------------------------------------
	@ember-data/serializer/json                       | 7.36 KB   | 1.54 KB    | 36.0          | 4.4
	@ember-data/serializer/-private                   | 5.60 KB   | 1.17 KB    | 27.4          | 3.3
	@ember-data/serializer/json-api                   | 3.79 KB   | 811.58 B   | 18.6          | 2.2
	@ember-data/serializer/rest                       | 2.87 KB   | 613.55 B   | 14.0          | 1.7
	@ember-data/serializer/index                      | 611.00 B  | 127.77 B   | 2.9           | 0.4
	@ember-data/serializer/transform                  | 215.00 B  | 44.96 B    | 1.0           | 0.1

Package: @ember-data/adapter
┌──────────────┬────────────┐
│   (index)    │   Values   │
├──────────────┼────────────┤
│    bytes     │ '15.93 KB' │
│  compressed  │ '3.33 KB'  │
│ % Of Library │   '9.4'    │
└──────────────┴────────────┘
	Module                                            | Bytes     | Compressed | % of Package  | % Of Library
	-----------------------------------------------------------------------------------------------------
	@ember-data/adapter/rest                          | 7.88 KB   | 1.65 KB    | 49.5          | 4.7
	@ember-data/adapter/-private                      | 3.79 KB   | 810.54 B   | 23.8          | 2.2
	@ember-data/adapter/error                         | 1.82 KB   | 390.21 B   | 11.4          | 1.1
	@ember-data/adapter/index                         | 1.41 KB   | 302.38 B   | 8.9           | 0.8
	@ember-data/adapter/json-api                      | 1.03 KB   | 221.45 B   | 6.5           | 0.6

Package: ember-data
┌──────────────┬───────────┐
│   (index)    │  Values   │
├──────────────┼───────────┤
│    bytes     │ '8.55 KB' │
│  compressed  │ '1.79 KB' │
│ % Of Library │   '5.1'   │
└──────────────┴───────────┘
	Module                                            | Bytes     | Compressed | % of Package  | % Of Library
	-----------------------------------------------------------------------------------------------------
	ember-data/index                                  | 2.08 KB   | 444.58 B   | 24.3          | 1.2
	ember-data/-private                               | 1.67 KB   | 357.59 B   | 19.5          | 1.0
	ember-data/adapters/errors                        | 1.19 KB   | 254.49 B   | 13.9          | 0.7
	ember-data/setup-container                        | 368.00 B  | 76.95 B    | 4.2           | 0.2
	ember-data/relationships                          | 318.00 B  | 66.49 B    | 3.6           | 0.2
	ember-data/serializers/embedded-records-mixin     | 274.00 B  | 57.29 B    | 3.1           | 0.2
	ember-data/serializers/json-api                   | 251.00 B  | 52.48 B    | 2.9           | 0.1
	ember-data/adapters/json-api                      | 245.00 B  | 51.23 B    | 2.8           | 0.1
	ember-data/serializers/json                       | 243.00 B  | 50.81 B    | 2.8           | 0.1
	ember-data/serializers/rest                       | 243.00 B  | 50.81 B    | 2.8           | 0.1
	ember-data/transform                              | 241.00 B  | 50.39 B    | 2.8           | 0.1
	ember-data/adapters/rest                          | 237.00 B  | 49.56 B    | 2.7           | 0.1
	ember-data/serializer                             | 232.00 B  | 48.51 B    | 2.7           | 0.1
	ember-data/adapter                                | 226.00 B  | 47.26 B    | 2.6           | 0.1
	ember-data/model                                  | 222.00 B  | 46.42 B    | 2.5           | 0.1
	ember-data/store                                  | 222.00 B  | 46.42 B    | 2.5           | 0.1
	ember-data/attr                                   | 218.00 B  | 45.58 B    | 2.5           | 0.1
	ember-data/version                                | 161.00 B  | 33.66 B    | 1.8           | 0.1

Package: ember-inflector
┌──────────────┬───────────┐
│   (index)    │  Values   │
├──────────────┼───────────┤
│    bytes     │ '6.67 KB' │
│  compressed  │ '1.39 KB' │
│ % Of Library │   '4.0'   │
└──────────────┴───────────┘
	Module                                            | Bytes     | Compressed | % of Package  | % Of Library
	-----------------------------------------------------------------------------------------------------
	ember-inflector/lib/system/inflector              | 2.96 KB   | 634.46 B   | 44.4          | 1.8
	ember-inflector/lib/system/inflections            | 1.59 KB   | 340.23 B   | 23.8          | 0.9
	ember-inflector/lib/system                        | 471.00 B  | 98.49 B    | 6.9           | 0.3
	ember-inflector/index                             | 379.00 B  | 79.25 B    | 5.6           | 0.2
	ember-inflector/lib/helpers/pluralize             | 369.00 B  | 77.16 B    | 5.4           | 0.2
	ember-inflector/lib/utils/make-helper             | 332.00 B  | 69.42 B    | 4.9           | 0.2
	ember-inflector/lib/system/string                 | 318.00 B  | 66.49 B    | 4.7           | 0.2
	ember-inflector/lib/helpers/singularize           | 296.00 B  | 61.89 B    | 4.3           | 0.2

Package: @ember-data/debug
┌──────────────┬────────────┐
│   (index)    │   Values   │
├──────────────┼────────────┤
│    bytes     │ '2.77 KB'  │
│  compressed  │ '592.43 B' │
│ % Of Library │   '1.6'    │
└──────────────┴────────────┘
	Module                                            | Bytes     | Compressed | % of Package  | % Of Library
	-----------------------------------------------------------------------------------------------------
	@ember-data/debug/index                           | 2.27 KB   | 486.82 B   | 82.2          | 1.3
	@ember-data/debug/setup                           | 505.00 B  | 105.60 B   | 17.8          | 0.3

Modern Builds (No Rollup)

☑️ EmberData has not changed in size

If any packages had changed sizes they would be listed here.

Changeset



Full Asset Analysis (Modern)

Asset Size Report
=================


Library: EmberData
┌────────────┬──────────┐
│  (index)   │  Values  │
├────────────┼──────────┤
│   bytes    │ '0.00 B' │
│ compressed │ '1.00 B' │
│  packages  │    0     │
│  modules   │    0     │
└────────────┴──────────┘

@github-actions
Copy link

github-actions bot commented Aug 17, 2022

Performance Report for 5a75092

Scenario - basic-record-materialization: ☑️ Performance is stable

☑️ duration
phase no difference [-47ms to 43ms]
☑️ Phase [navigationStart] => [start-data-generation]
phase no difference [-3ms to 5ms]
☑️ Phase [start-data-generation] => [start-push-payload]
phase no difference [0ms to 6ms]
☑️ Phase [start-push-payload] => [start-peek-records]
phase no difference [-42ms to 11ms]
☑️ Phase [start-peek-records] => [start-record-materialization]
phase no difference [-3ms to 2ms]
☑️ Phase [start-record-materialization] => [end-record-materialization]
phase no difference [-8ms to 31ms]
☑️ Phase [end-record-materialization] => [Test End]
phase no difference [-3ms to 8ms]

Scenario - relationship-materialization-simple: ☑️ Performance is stable

☑️ duration
phase no difference [-19ms to 15ms]
☑️ Phase [navigationStart] => [start-find-all]
phase no difference [-4ms to 5ms]
☑️ Phase [start-find-all] => [start-materialization]
phase no difference [-9ms to 3ms]
☑️ Phase [start-materialization] => [end-materialization]
phase no difference [-7ms to 1ms]
☑️ Phase [end-materialization] => [Test End]
phase no difference [0ms to 0ms]

Scenario - relationship-materialization-complex: ☑️ Performance is stable

☑️ duration
phase no difference [-93ms to 40ms]
☑️ Phase [navigationStart] => [start-data-generation]
phase no difference [-3ms to 5ms]
☑️ Phase [start-data-generation] => [start-push-payload]
phase no difference [-1ms to 1ms]
☑️ Phase [start-push-payload] => [start-peek-records]
phase no difference [-15ms to 11ms]
⚠️ Phase [start-peek-records] => [start-record-materialization]
phase estimated regression +2ms [0ms to 3ms] OR +4.16% [0.32% to 7.59%]
☑️ Phase [start-record-materialization] => [start-relationship-materialization]
phase no difference [-10ms to 3ms]
✅ Phase [start-relationship-materialization] => [end-relationship-materialization]
phase estimated improvement -53ms [-105ms to -5ms] OR -1.36% [-2.71% to -0.12%]
⚠️ Phase [end-relationship-materialization] => [Test End]
phase estimated regression +18ms [8ms to 28ms] OR +3% [1.38% to 4.62%]

Scenario - unload: ☑️ Performance is stable

☑️ duration
phase no difference [-13ms to 20ms]
☑️ Phase [navigationStart] => [start-push-payload]
phase no difference [-4ms to 7ms]
☑️ Phase [start-push-payload] => [start-unload-records]
phase no difference [-7ms to 6ms]
⚠️ Phase [start-unload-records] => [end-unload-records]
phase estimated regression +4ms [1ms to 7ms] OR +1.88% [0.54% to 3.07%]
☑️ Phase [end-unload-records] => [Test End]
phase no difference [0ms to 0ms]

Scenario - unload-all: ☑️ Performance is stable

☑️ duration
phase no difference [-12ms to 44ms]
☑️ Phase [navigationStart] => [start-push-payload]
phase no difference [-8ms to 6ms]
☑️ Phase [start-push-payload] => [start-materialization]
phase no difference [-6ms to 22ms]
☑️ Phase [start-materialization] => [start-unload-all]
phase no difference [-7ms to 5ms]
⚠️ Phase [start-unload-all] => [end-unload-all]
phase estimated regression +13ms [7ms to 19ms] OR +2.2% [1.23% to 3.15%]
☑️ Phase [end-unload-all] => [Test End]
phase no difference [0ms to 1ms]

Scenario - destroy: ☑️ Performance is stable

☑️ duration
phase no difference [-12ms to 20ms]
☑️ Phase [navigationStart] => [start-push-payload]
phase no difference [-6ms to 5ms]
☑️ Phase [start-push-payload] => [start-destroy-records]
phase no difference [-6ms to 8ms]
⚠️ Phase [start-destroy-records] => [end-destroy-records]
phase estimated regression +4ms [2ms to 5ms] OR +1.54% [0.81% to 2.33%]
☑️ Phase [end-destroy-records] => [Test End]
phase no difference [0ms to 0ms]

Scenario - add-children: ☑️ Performance is stable

☑️ duration
phase no difference [-13ms to 14ms]
☑️ Phase [navigationStart] => [start-push-initial-payload]
phase no difference [-8ms to 8ms]
☑️ Phase [start-push-initial-payload] => [start-push-update-payload]
phase no difference [-5ms to 1ms]
☑️ Phase [start-push-update-payload] => [end-push-update-payload]
phase no difference [-4ms to 2ms]
⚠️ Phase [end-push-update-payload] => [Test End]
phase estimated regression +4ms [3ms to 4ms] OR +6.88% [5.7% to 8.03%]

Scenario - unused-relationships: ☑️ Performance is stable

☑️ duration
phase no difference [-11ms to 25ms]
☑️ Phase [navigationStart] => [start-push-payload]
phase no difference [-3ms to 8ms]
☑️ Phase [start-push-payload] => [end-push-payload]
phase no difference [-9ms to 9ms]
⚠️ Phase [end-push-payload] => [Test End]
phase estimated regression +4ms [3ms to 5ms] OR +7.4% [5.93% to 8.78%]

@runspired runspired mentioned this pull request Aug 18, 2022
8 tasks
@runspired runspired linked an issue Aug 18, 2022 that may be closed by this pull request
@runspired runspired changed the title adds test for #8125 fix: backgroundReloads should not affect recordIsLoaded (#8125) Aug 18, 2022
@runspired runspired added the 🏷️ bug This PR primarily fixes a reported issue label Aug 18, 2022
@runspired runspired merged commit 51b724b into master Aug 18, 2022
@delete-merged-branch delete-merged-branch bot deleted the runspired/fix-8125 branch August 18, 2022 20:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🎯 canary PR is targeting canary (default) 🏷️ bug This PR primarily fixes a reported issue 🏷️ test This PR primarily adds tests for a feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

findRecord returns null on second call
1 participant