Skip to content

Commit

Permalink
fix(au-slot): separate parent scope selection from host scope selecti…
Browse files Browse the repository at this point in the history 
…on (#1961)

[skip ci]
  • Loading branch information
@bigopon
bigopon committed May 6, 2024
1 parent eaf2cd7 commit ff605fb
Show file tree
Hide file tree
Showing 8 changed files with 190 additions and 270 deletions.
28 changes: 26 additions & 2 deletions packages/__tests__/src/3-runtime-html/au-slot.spec.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2083,6 +2083,30 @@ describe('3-runtime-html/au-slot.spec.tsx', function () {
assertHtml('p > a', 'hello');
});

// bug reported by @MaxB on Discord
// https://discord.com/channels/448698263508615178/1236855768058302526
it('rightly chooses the scope for projected content', function () {
@customElement({
name: 'mdc-lookup',
template:
'<div repeat.for="option of options">' +
'<mdc-option>' +
'<au-slot>${$parent.option} -- ${option}</au-slot>' +
'</mdc-option>' +
'</div>',
})
class MdcLookup {
options = ['option1'];
}

@customElement({ name: 'mdc-option', template: '<au-slot></au-slot>' })
class MdcOption {}

const { assertHtml } = createFixture('<mdc-lookup></mdc-lookup>', class {}, [MdcLookup, MdcOption]);

assertHtml('<mdc-lookup><div><mdc-option>option1 -- option1</mdc-option></div></mdc-lookup>', { compact: true });
});

describe('with multi layers of repeaters', function () {
// au-slot creates a layer of scope
// making $parent from the inner repeater not reaching to the outer repeater
Expand Down Expand Up @@ -2137,7 +2161,7 @@ describe('3-runtime-html/au-slot.spec.tsx', function () {
});
});

// bug discorverd by @MaxB on Discord
// bug discorvered by @MaxB on Discord
// https://discord.com/channels/448698263508615178/448699089513611266/1234665951467929666
it('passes $host value through 1 layer of <au-slot>', function () {

Expand Down Expand Up @@ -2170,7 +2194,7 @@ describe('3-runtime-html/au-slot.spec.tsx', function () {
class MyApp {},
[MdcLookup, MdcFilter, MdcOption, class {
static $au = { type: 'value-converter', name: 'json' };
toView = (v: unknown, tag = '') => { console.log({ ctor: `${tag}:${v?.constructor.name ?? '<undefined>'}` }); return v; };
toView = (v: unknown) => v;
}]
);

Expand Down
106 changes: 38 additions & 68 deletions packages/runtime-html/src/compiler/template-compiler.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,8 @@ import {
type IRegistry
} from '@aurelia/kernel';
import {
BindingCommand,
BindingCommandDefinition,
type BindingCommandInstance,
type IAttributeBindablesInfo,
type IElementBindablesInfo,
IBindablesInfoResolver,
IResourceResolver,
TemplateCompiler,
} from '@aurelia/template-compiler';
Expand All @@ -31,21 +27,48 @@ export const RuntimeTemplateCompilerImplementation: IRegistry = {
container.register(
TemplateCompiler,
AttrMapper,
BindablesInfoResolver,
ResourceResolver,
);
}
};

class BindablesInfoResolver implements IBindablesInfoResolver<CustomElementDefinition, CustomAttributeDefinition> {
public static register = /*@__PURE__*/ createImplementationRegister(IBindablesInfoResolver);
class BindablesInfo {
public constructor(
public readonly attrs: Record<string, BindableDefinition>,
public readonly bindables: Record<string, BindableDefinition>,
public readonly primary: BindableDefinition | null,
) {}
}

class ResourceResolver implements IResourceResolver<CustomElementDefinition, CustomAttributeDefinition> {
public static register = /*@__PURE__*/ createImplementationRegister(IResourceResolver);

/** @internal */
private readonly _resourceCache = new WeakMap<IContainer, RecordCache>();

public el(c: IContainer, name: string): CustomElementDefinition | null {
let record = this._resourceCache.get(c);
if (record == null) {
this._resourceCache.set(c, record = new RecordCache());
}
return name in record._element ? record._element[name] : (record._element[name] = CustomElement.find(c, name));
}

public attr(c: IContainer, name: string): CustomAttributeDefinition | null {
let record = this._resourceCache.get(c);
if (record == null) {
this._resourceCache.set(c, record = new RecordCache());
}
return name in record._attr ? record._attr[name] : (record._attr[name] = CustomAttribute.find(c, name));
}

/** @internal */
private readonly _cache = new WeakMap<CustomElementDefinition | CustomAttributeDefinition, BindablesInfo>();
private readonly _bindableCache = new WeakMap<CustomElementDefinition | CustomAttributeDefinition, BindablesInfo>();

public get(def: CustomAttributeDefinition): IAttributeBindablesInfo;
public get(def: CustomElementDefinition): IElementBindablesInfo;
public get(def: CustomAttributeDefinition | CustomElementDefinition): IAttributeBindablesInfo | IElementBindablesInfo {
let info = this._cache.get(def);
public bindables(def: CustomAttributeDefinition): IAttributeBindablesInfo;
public bindables(def: CustomElementDefinition): IElementBindablesInfo;
public bindables(def: CustomAttributeDefinition | CustomElementDefinition): IAttributeBindablesInfo | IElementBindablesInfo {
let info = this._bindableCache.get(def);
if (info == null) {
const bindables = def.bindables;
const attrs = createLookup<BindableDefinition>();
Expand Down Expand Up @@ -81,66 +104,13 @@ class BindablesInfoResolver implements IBindablesInfoResolver<CustomElementDefin
);
}

this._cache.set(def, info = new BindablesInfo(attrs, bindables, primary ?? null));
this._bindableCache.set(def, info = new BindablesInfo(attrs, bindables, primary ?? null));
}
return info;
}
}

class BindablesInfo {
public constructor(
public readonly attrs: Record<string, BindableDefinition>,
public readonly bindables: Record<string, BindableDefinition>,
public readonly primary: BindableDefinition | null,
) {}
}

class ResourceResolver implements IResourceResolver<CustomElementDefinition, CustomAttributeDefinition> {
public static register = /*@__PURE__*/ createImplementationRegister(IResourceResolver);

private readonly _resourceCache = new WeakMap<IContainer, RecordCache>();
private readonly _commandCache = new WeakMap<IContainer, Record<string, BindingCommandInstance | null>>();

public el(c: IContainer, name: string): CustomElementDefinition | null {
let record = this._resourceCache.get(c);
if (record == null) {
this._resourceCache.set(c, record = new RecordCache());
}
return name in record.element ? record.element[name] : (record.element[name] = CustomElement.find(c, name));
}

public attr(c: IContainer, name: string): CustomAttributeDefinition | null {
let record = this._resourceCache.get(c);
if (record == null) {
this._resourceCache.set(c, record = new RecordCache());
}
return name in record.attr ? record.attr[name] : (record.attr[name] = CustomAttribute.find(c, name));
}

public command(c: IContainer, name: string): BindingCommandInstance | null {
let commandInstanceCache = this._commandCache.get(c);
if (commandInstanceCache == null) {
this._commandCache.set(c, commandInstanceCache = createLookup());
}
let result = commandInstanceCache[name];
if (result === void 0) {
let record = this._resourceCache.get(c);
if (record == null) {
this._resourceCache.set(c, record = new RecordCache());
}

const commandDef = name in record.command ? record.command[name] : (record.command[name] = BindingCommand.find(c, name));
if (commandDef == null) {
throw createMappedError(ErrorNames.compiler_unknown_binding_command, name);
}
commandInstanceCache[name] = result = BindingCommand.get(c, name);
}
return result;
}
}

class RecordCache {
public element = createLookup<CustomElementDefinition | null>();
public attr = createLookup<CustomAttributeDefinition | null>();
public command = createLookup<BindingCommandDefinition | null>();
public _element = createLookup<CustomElementDefinition | null>();
public _attr = createLookup<CustomAttributeDefinition | null>();
}
10 changes: 7 additions & 3 deletions packages/runtime-html/src/resources/custom-elements/au-slot.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ export class AuSlot implements ICustomElementViewModel, IAuSlot {
// we won't find the information in the hydration context hierarchy <MyApp>/<S3>
// as it's a flat wysiwyg structure based on the template html
//
// since we are construction the projection (2) view based on the
// since we are constructing the projection (2) view based on the
// container of <my-app>, we need to pre-register all information stored
// in projection (1) into the container created for the projection (2) view
// =============================
Expand Down Expand Up @@ -166,6 +166,10 @@ export class AuSlot implements ICustomElementViewModel, IAuSlot {
_initiator: IHydratedController,
parent: IHydratedParentController,
): void | Promise<void> {
this._parentScope = parent.scope;

// The following block finds the real host scope for the content of this <au-slot>
//
// if this <au-slot> was created by another au slot, the controller hierarchy will be like this:
// C(au-slot)#1 --> C(synthetic)#1 --> C(au-slot)#2 --> C(synthetic)#2
//
Expand All @@ -176,7 +180,7 @@ export class AuSlot implements ICustomElementViewModel, IAuSlot {
while (parent.vmKind === 'synthetic' && parent.parent?.viewModel instanceof AuSlot) {
parent = parent.parent.parent as IHydratedParentController;
}
this._parentScope = parent.scope;
const host = parent.scope.bindingContext;

let outerScope: Scope;
if (this._hasProjection) {
Expand All @@ -187,7 +191,7 @@ export class AuSlot implements ICustomElementViewModel, IAuSlot {
// - override context has the $host pointing to inner scope binding context
outerScope = this._hdrContext.controller.scope.parent!;
(this._outerScope = Scope.fromParent(outerScope, outerScope.bindingContext))
.overrideContext.$host = this.expose ?? this._parentScope.bindingContext;
.overrideContext.$host = this.expose ?? host;
}
}

Expand Down
94 changes: 53 additions & 41 deletions packages/runtime-html/src/templating/rendering.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,8 @@ export class Rendering implements IRendering {
private readonly _fragmentCache: WeakMap<CustomElementDefinition, DocumentFragment | null> = new WeakMap();
/** @internal */
private readonly _empty: INodeSequence;
/** @internal */
private readonly _marker: Node;

public get renderers(): Record<string, IRenderer> {
return this._renderers ??= this._ctn.getAll(IRenderer, false).reduce((all, r) => {
Expand All @@ -67,10 +69,11 @@ export class Rendering implements IRendering {

public constructor() {
const ctn = this._ctn = resolve(IContainer).root;
this._platform = ctn.get(IPlatform);
const p = this._platform = ctn.get(IPlatform);
this._exprParser= ctn.get(IExpressionParser);
this._observerLocator = ctn.get(IObserverLocator);
this._empty = new FragmentNodeSequence(this._platform, this._platform.document.createDocumentFragment());
this._marker = p.document.createElement('au-m');
this._empty = new FragmentNodeSequence(p, p.document.createDocumentFragment());
}

public compile(
Expand Down Expand Up @@ -191,52 +194,61 @@ export class Rendering implements IRendering {
}
}

/** @internal */
private _marker() {
return this._platform.document.createElement('au-m');
}

/** @internal */
private _transformMarker(fragment: Node | null) {
if (fragment == null) {
return null;
}
let parent: Node = fragment;
let current: Node | null | undefined = parent.firstChild;
let next: Node | null | undefined = null;

while (current != null) {
if (current.nodeType === 8 && current.nodeValue === 'au*') {
next = current.nextSibling!;
parent.removeChild(current);
parent.insertBefore(this._marker(), next);
if (next.nodeType === 8) {
current = next.nextSibling;
// todo: maybe validate?
} else {
current = next;
}
}

next = current?.firstChild;
if (next == null) {
next = current?.nextSibling;
if (next == null) {
current = parent.nextSibling;
parent = parent.parentNode!;
// needs to keep walking up all the way til a valid next node
while (current == null && parent != null) {
current = parent.nextSibling;
parent = parent.parentNode!;
}
} else {
current = next;
}
} else {
parent = current!;
current = next;
const walker = this._platform.document.createTreeWalker(fragment, /* NodeFilter.SHOW_COMMENT */ 128);
let currentNode: Node | null;
while ((currentNode = walker.nextNode()) != null) {
if (currentNode.nodeValue === 'au*') {
currentNode.parentNode!.replaceChild(walker.currentNode = this._marker.cloneNode(), currentNode);
}
}
return fragment;
// below is a homemade "comment query selector that seems to be as efficient as the TreeWalker
// also it works with very minimal set of APIs (.nextSibling, .parentNode, .insertBefore, .removeChild)
// while TreeWalker maynot be always available in platform that we may potentially support
//
// so leaving it here just in case we need it again, TreeWalker is slightly less code

// let parent: Node = fragment;
// let current: Node | null | undefined = parent.firstChild;
// let next: Node | null | undefined = null;

// while (current != null) {
// if (current.nodeType === 8 && current.nodeValue === 'au*') {
// next = current.nextSibling!;
// parent.removeChild(current);
// parent.insertBefore(this._marker(), next);
// if (next.nodeType === 8) {
// current = next.nextSibling;
// // todo: maybe validate?
// } else {
// current = next;
// }
// }

// next = current?.firstChild;
// if (next == null) {
// next = current?.nextSibling;
// if (next == null) {
// current = parent.nextSibling;
// parent = parent.parentNode!;
// // needs to keep walking up all the way til a valid next node
// while (current == null && parent != null) {
// current = parent.nextSibling;
// parent = parent.parentNode!;
// }
// } else {
// current = next;
// }
// } else {
// parent = current!;
// current = next;
// }
// }
// return fragment;
}
}

0 comments on commit ff605fb

Please sign in to comment.