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 component cache for polymorphic field re-renders #2189

Merged
merged 11 commits into from
Mar 6, 2025
Merged
4 changes: 2 additions & 2 deletions packages/base/card-api.gts
Original file line number Diff line number Diff line change
Expand Up @@ -350,12 +350,12 @@ function callSerializeHook(

function cardTypeFor(
field: Field<typeof BaseDef>,
boxedElement: Box<BaseDef>,
boxedElement?: Box<BaseDef>,
): typeof BaseDef {
if (primitive in field.card) {
return field.card;
}
if (boxedElement.value == null) {
if (boxedElement === undefined || boxedElement.value == null) {
return field.card;
}
return Reflect.getPrototypeOf(boxedElement.value)!
Expand Down
32 changes: 14 additions & 18 deletions packages/base/field-component.gts
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,11 @@ export class PermissionsConsumer extends Component<PermissionsConsumerSignature>

const componentCache = initSharedState(
'componentCache',
() => new WeakMap<Box<BaseDef>, BoxComponent>(),
() =>
new WeakMap<
Box<BaseDef>,
{ component: BoxComponent; cardOrField: typeof BaseDef }
>(),
);

export function getBoxComponent(
Expand All @@ -130,8 +134,8 @@ export function getBoxComponent(
// the componentCodeRef is only set on the server during card prerendering,
// it should have no effect on component stability
let stable = componentCache.get(model);
if (stable) {
return stable;
if (stable?.cardOrField === cardOrField) {
return stable.component;
}
function determineFormats(
userFormat: Format | undefined,
Expand Down Expand Up @@ -364,9 +368,13 @@ export function getBoxComponent(
let externalFields = fieldsComponentsFor(component, model);

// This cast is safe because we're returning a proxy that wraps component.
stable = externalFields as unknown as typeof component;
stable = {
component: externalFields as unknown as typeof component,
cardOrField: cardOrField,
};

componentCache.set(model, stable);
return stable;
return stable.component;
}

function defaultFieldFormats(containingFormat: Format): FieldFormats {
Expand All @@ -386,10 +394,6 @@ function fieldsComponentsFor<T extends BaseDef>(
target: object,
model: Box<T>,
): FieldsTypeFor<T> {
// This is a cache of the fields we've already created components for
// so that they do not get recreated
let stableComponents = new Map<string, BoxComponent>();

return new Proxy(target, {
get(target, property, received) {
if (
Expand All @@ -401,11 +405,6 @@ function fieldsComponentsFor<T extends BaseDef>(
return Reflect.get(target, property, received);
}

let stable = stableComponents.get(property);
if (stable) {
return stable;
}

let modelValue = model.value as T; // TS is not picking up the fact we already filtered out nulls and undefined above
let maybeField: Field<BaseDefConstructor> | undefined = getField(
modelValue.constructor,
Expand All @@ -416,10 +415,7 @@ function fieldsComponentsFor<T extends BaseDef>(
return Reflect.get(target, property, received);
}
let field = maybeField;

let result = field.component(model as unknown as Box<BaseDef>);
stableComponents.set(property, result);
return result;
return field.component(model as unknown as Box<BaseDef>);
},
getPrototypeOf() {
// This is necessary for Ember to be able to locate the template associated
Expand Down
25 changes: 23 additions & 2 deletions packages/base/links-to-many-component.gts
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ import {
} from '@cardstack/boxel-ui/modifiers';

import { action } from '@ember/object';
import { initSharedState } from './shared-state';

interface Signature {
Element: HTMLElement;
Expand Down Expand Up @@ -151,7 +152,7 @@ class LinksToManyStandardEditor extends GlimmerComponent<LinksToManyStandardEdit

@action
setItems(items: any) {
(this.args.model.value as any)[this.args.field.name] = items;
this.args.arrayField.set(items);
}

<template>
Expand Down Expand Up @@ -380,6 +381,14 @@ function shouldRenderEditor(
) {
return (format ?? defaultFormat) === 'edit' && !isComputed;
}
const componentCache = initSharedState(
'linksToManyComponentCache',
() =>
new WeakMap<
Box<BaseDef[]>,
{ component: BoxComponent; cardOrField: typeof BaseDef }
>(),
);

export function getLinksToManyComponent({
model,
Expand All @@ -395,6 +404,11 @@ export function getLinksToManyComponent({
boxedElement: Box<BaseDef>,
): typeof BaseDef;
}): BoxComponent {
let cardOrField = cardTypeFor(field, arrayField.children[0]);
let stable = componentCache.get(arrayField);
if (stable?.cardOrField === cardOrField) {
return stable.component;
}
let getComponents = () =>
arrayField.children.map((child) =>
getBoxComponent(cardTypeFor(field, child), child, field),
Expand Down Expand Up @@ -465,7 +479,7 @@ export function getLinksToManyComponent({
</style>
</template>
};
return new Proxy(linksToManyComponent, {
let proxy = new Proxy(linksToManyComponent, {
get(target, property, received) {
// proxying the bare minimum of an Array in order to render within a
// template. add more getters as necessary...
Expand All @@ -490,6 +504,13 @@ export function getLinksToManyComponent({
return linksToManyComponent;
},
});
stable = {
component: proxy as unknown as BoxComponent,
cardOrField: cardOrField,
};

componentCache.set(arrayField, stable);
return stable.component;
}

function myLoader(): Loader {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
{
"data": {
"type": "card",
"attributes": {
"specialField": {
"firstName": "Jersey"
},
"title": null,
"description": null,
"thumbnailURL": null
},
"meta": {
"adoptsFrom": {
"module": "../polymorphic-field",
"name": "CardWithSpecialFields"
}
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
{
"data": {
"type": "card",
"attributes": {
"specialField": {
"firstName": "Boboy"
},
"title": null,
"description": null,
"thumbnailURL": null
},
"meta": {
"adoptsFrom": {
"module": "../polymorphic-field",
"name": "CardWithSpecialFields"
}
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
{
"data": {
"type": "card",
"attributes": {
"specialField": {
"firstName": "Milo"
},
"title": null,
"description": null,
"thumbnailURL": null
},
"relationships": {
"cardsWithSpecialFields.0": {
"links": {
"self": "../CardWithSpecialFields/1796a22a-0d8a-4aae-a320-6a82d497fc7e"
}
},
"cardsWithSpecialFields.1": {
"links": {
"self": "../CardWithSpecialFields/c9442771-211f-4e1e-b767-4a8e0052de05"
}
}
},
"meta": {
"adoptsFrom": {
"module": "../polymorphic-field",
"name": "PolymorphicFieldExample"
}
}
}
}
99 changes: 99 additions & 0 deletions packages/experiments-realm/polymorphic-field.gts
Original file line number Diff line number Diff line change
@@ -0,0 +1,99 @@
import {
Component,
CardDef,
field,
contains,
StringField,
FieldDef,
linksToMany,
} from 'https://cardstack.com/base/card-api';
import { on } from '@ember/modifier';

export class TestField extends FieldDef {
static displayName = 'TestField';
@field firstName = contains(StringField);

static fitted = class Fitted extends Component<typeof this> {
<template>
<div data-test-baseclass>
BaseClass
<@fields.firstName />
</div>
</template>
};

static embedded = class Embedded extends Component<typeof this> {
<template>
<div data-test-baseclass>
Embedded BaseClass
<@fields.firstName />
</div>
</template>
};
}
export class SubTestField extends TestField {
static displayName = 'SubTestField';

static fitted = class Fitted extends Component<typeof this> {
<template>
<div data-test-subclass>
SubClass
<@fields.firstName />
</div>
</template>
};

static embedded = class Embedded extends Component<typeof this> {
<template>
<div data-test-subclass>
Embedded SubClass
<@fields.firstName />
</div>
</template>
};

static edit = class Edit extends Component<typeof this> {
<template>
<div data-test-edit>
Edit
<@fields.firstName />
</div>
</template>
};
}

export class CardWithSpecialFields extends CardDef {
static displayName = 'CardWithSpecialFields';
@field specialField = contains(TestField);

static fitted = class Fitted extends Component<typeof this> {
setSubclass = () => {
this.args.model.specialField = new SubTestField({});
};
<template>
<div data-test-card-with-special-fields>
<@fields.specialField />
<button {{on 'click' this.setSubclass}} data-test-set-subclass>Set
Subclass From Inside</button>
</div>
</template>
};
}

export class PolymorphicFieldExample extends CardDef {
static displayName = 'PolymorphicFieldExample';
@field specialField = contains(TestField);
@field cardsWithSpecialFields = linksToMany(() => CardWithSpecialFields);

static isolated = class Isolated extends Component<typeof this> {
setSubclass = () => {
this.args.model.specialField = new SubTestField({});
};
<template>
<button {{on 'click' this.setSubclass}} data-test-set-subclass>Set
Subclass From Outside</button>
<@fields.specialField />
<@fields.cardsWithSpecialFields />
</template>
};
}
Loading