-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
[Cleanup]: Remove RECORD_ARRAY_MANAGER_IDENTIFIERS #7394
Conversation
Performance Report for eb2a192 Relationship Analysis
|
Asset Size Report for eb2a192 IE11 Builds EmberData shrank by 71.0 B (-22.0 B compressed)If any packages had changed sizes they would be listed here. Changeset
Full Asset Analysis (IE11)
Modern Builds EmberData shrank by 71.0 B (-22.0 B compressed)If any packages had changed sizes they would be listed here. Changeset
Full Asset Analysis (Modern)
Modern Builds (No Rollup) EmberData shrank by 122.0 B (27.0 B compressed)If any packages had changed sizes they would be listed here. Changeset
Full Asset Analysis (Modern)
|
A few possible paths here. 1 - we could consider this feature flag done and any further work to remove internalModel could be under new feature flags. 2 - close this PR and find more spots to remove internalModel (core store?). One reason we might consider this feature flag completed is because we have ✂️ the use of |
This reverts commit 991e96f.
@@ -177,6 +177,5 @@ export default { | |||
REQUEST_SERVICE: null, | |||
CUSTOM_MODEL_CLASS: null, | |||
FULL_LINKS_ON_RELATIONSHIPS: true, | |||
RECORD_ARRAY_MANAGER_IDENTIFIERS: true, | |||
REMOVE_RECORD_ARRAY_MANAGER_LEGACY_COMPAT: null, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We probably want to keep the flag exposed
@@ -38,7 +38,6 @@ export const REQUEST_SERVICE = featureValue(FEATURES.REQUEST_SERVICE); | |||
export const IDENTIFIERS = featureValue(FEATURES.IDENTIFIERS); | |||
export const CUSTOM_MODEL_CLASS = featureValue(FEATURES.CUSTOM_MODEL_CLASS); | |||
export const FULL_LINKS_ON_RELATIONSHIPS = featureValue(FEATURES.FULL_LINKS_ON_RELATIONSHIPS); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We probably want to keep the flag exposed
@@ -32,15 +32,10 @@ function isResourceIdentiferWithRelatedLinks( | |||
return value && value.links && value.links.related; | |||
} | |||
|
|||
// TODO: simplify after 3.23 release and only store identifier | |||
export const REFERENCE_CACHE = new WeakMap<Reference, InternalModel | StableRecordIdentifier>(); | |||
export const REFERENCE_CACHE = new WeakMap<Reference, StableRecordIdentifier>(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the cleanup
@snewcomer we should use a new feature flag for new units of work, so they are easier to back out of. A lot of cleanup might not even need it, depending on how risky it is |
This flag helped us move from using Internal Models to Identifiers. With the 3.23 release, we can remove this flag.