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.

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

refactor(engine-core): replaces vm.context wire connecting/disconnecting #2130

Closed
wants to merge 5 commits into from
Closed

refactor(engine-core): replaces vm.context wire connecting/disconnecting #2130

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
6710b9b
refactor(engine-core): replaces vm.context wire connecting/disconnecting
jodarove Dec 10, 2020
ba01cbd
refactor: connector, config watcher, and context watcher into objects
jodarove Dec 14, 2020
8777fd6
wip: missing array.push
jodarove Dec 14, 2020
b1778cc
wip: rename internal wire interface
jodarove Dec 14, 2020
cc0030f
wip: address review comments
jodarove Dec 16, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
14 changes: 8 additions & 6 deletions packages/@lwc/engine-core/src/framework/vm.ts
View file
Open in desktop
Expand Up @@ -66,6 +66,11 @@ export enum VMState {
disconnected,
}

export interface WireConnector {
jodarove marked this conversation as resolved.
Show resolved Hide resolved
connect: () => void;
disconnect: () => void;
}

export interface Context {
/** The attribute name used on the host element to scope the style. */
hostAttribute: string | undefined;
Expand All @@ -76,10 +81,8 @@ export interface Context {
/** Object used by the template function to store information that can be reused between
* different render cycle of the same template. */
tplCache: TemplateCache;
/** List of wire hooks that are invoked when the component gets connected. */
wiredConnecting: Array<() => void>;
/** List of wire hooks that are invoked when the component gets disconnected. */
wiredDisconnecting: Array<() => void>;
/** List of wire connectors attached to this component. */
wiredConnectors: WireConnector[];
}

export interface VM<N = HostNode, E = HostElement> {
Expand Down Expand Up @@ -275,8 +278,7 @@ export function createVM<HostNode, HostElement>(
shadowAttribute: undefined,
styleVNode: null,
tplCache: EmptyObject,
wiredConnecting: EmptyArray,
wiredDisconnecting: EmptyArray,
wiredConnectors: EmptyArray,
},

tro: null!, // Set synchronously after the VM creation.
Expand Down
217 changes: 118 additions & 99 deletions packages/@lwc/engine-core/src/framework/wiring.ts
View file
Open in desktop
Expand Up @@ -7,7 +7,8 @@
import { assert, isUndefined, ArrayPush, defineProperty, defineProperties } from '@lwc/shared';
import { ComponentInterface } from './component';
import { componentValueMutated, ReactiveObserver } from './mutation-tracker';
import { VM, runWithBoundaryProtection, VMState } from './vm';
import { VM, runWithBoundaryProtection, WireConnector } from './vm';
import { HostElement, Renderer } from './renderer';

const DeprecatedWiredElementHost = '$$DeprecatedWiredElementHostKey$$';
const DeprecatedWiredParamsMeta = '$$DeprecatedWiredParamsMetaKey$$';
Expand Down Expand Up @@ -108,57 +109,74 @@ function createConfigWatcher(
}

function createContextWatcher(
vm: VM,
wireDef: WireDef,
callbackWhenContextIsReady: (newContext: ContextValue) => void
) {
const { adapter } = wireDef;
adapter: WireAdapterConstructor,
elm: HostElement,
renderer: Renderer,
computeConfigAndUpdate: () => void
): ContextWatcher | undefined {
if (isUndefined(adapter.contextSchema)) {
return; // no config watcher required
}

const adapterContextToken = getAdapterToken(adapter);
if (isUndefined(adapterContextToken)) {
return; // no provider found, nothing to be done
}
const {
elm,
renderer,
context: { wiredConnecting, wiredDisconnecting },
} = vm;
// waiting for the component to be connected to formally request the context via the token
ArrayPush.call(wiredConnecting, () => {
// This event is responsible for connecting the host element with another
// element in the composed path that is providing contextual data. The provider
// must be listening for a special dom event with the name corresponding to the value of
// `adapterContextToken`, which will remain secret and internal to this file only to
// guarantee that the linkage can be forged.
const contextRegistrationEvent = new WireContextRegistrationEvent(adapterContextToken, {
setNewContext(newContext: ContextValue) {
// eslint-disable-next-line lwc-internal/no-invalid-todo
// TODO: dev-mode validation of config based on the adapter.contextSchema
callbackWhenContextIsReady(newContext);
},
setDisconnectedCallback(disconnectCallback: () => void) {
// adds this callback into the disconnect bucket so it gets disconnected from parent
// the the element hosting the wire is disconnected
ArrayPush.call(wiredDisconnecting, disconnectCallback);
},
});
renderer.dispatchEvent(elm, contextRegistrationEvent);
});

let isConnected = false;
let context: ContextValue | undefined;
const wiredDisconnecting: Array<() => void> = [];
jodarove marked this conversation as resolved.
Show resolved Hide resolved

return {
connect() {
isConnected = true;

// This event is responsible for connecting the host element with another
// element in the composed path that is providing contextual data. The provider
// must be listening for a special dom event with the name corresponding to the value of
// `adapterContextToken`, which will remain secret and internal to this file only to
// guarantee that the linkage can be forged.
const contextRegistrationEvent = new WireContextRegistrationEvent(adapterContextToken, {
setNewContext(newContext: ContextValue) {
// eslint-disable-next-line lwc-internal/no-invalid-todo
// TODO: dev-mode validation of config based on the adapter.contextSchema
if (context !== newContext) {
context = newContext;
// Note: when new context arrives, the config will be recomputed and pushed along side the new
// context, this is to preserve the identity characteristics, config should not have identity
// (ever), while context can have identity
if (isConnected) {
computeConfigAndUpdate();
}
}
},
setDisconnectedCallback(disconnectCallback: () => void) {
// adds this callback into the disconnect bucket so it gets disconnected from parent
// the the element hosting the wire is disconnected
jodarove marked this conversation as resolved.
Show resolved Hide resolved
ArrayPush.call(wiredDisconnecting, disconnectCallback);
},
});
renderer.dispatchEvent(elm, contextRegistrationEvent);
},
disconnect() {
isConnected = false;

for (let i = 0, n = wiredDisconnecting.length; i < n; i++) {
wiredDisconnecting[i]();
}
},
get contextValue() {
return context;
},
};
}

function createConnector(
vm: VM,
name: string,
wireDef: WireDef
): {
connector: WireAdapter;
computeConfigAndUpdate: () => void;
resetConfigWatcher: () => void;
} {
function createConnector(vm: VM, name: string, wireDef: WireDef): WireConnector {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The amount of closure passing makes the code hard to read. I would convert the connector, config watcher, and context watcher into 3 different classes and pass the object reference around. This would certainly make the code more readable and reduce the overall memory usage due to the closure creation.

const { method, adapter, configCallback, dynamic } = wireDef;
const hasDynamicParams = dynamic.length > 0;
const dataCallback = isUndefined(method)
? createFieldDataCallback(vm, name)
: createMethodDataCallback(vm, method);
let context: ContextValue | undefined;
let connector: WireAdapter;

// Workaround to pass the component element associated to this wire adapter instance.
Expand All @@ -179,6 +197,9 @@ function createConnector(
},
noop
);

let contextWatcher: ContextWatcher | undefined;

const updateConnectorConfig = (config: ConfigValue) => {
// every time the config is recomputed due to tracking,
// this callback will be invoked with the new computed config
Expand All @@ -188,7 +209,7 @@ function createConnector(
noop,
() => {
// job
connector.update(config, context);
connector.update(config, contextWatcher?.contextValue);
},
noop
);
Expand All @@ -202,27 +223,44 @@ function createConnector(
updateConnectorConfig
);

// if the adapter needs contextualization, we need to watch for new context and push it alongside the config
if (!isUndefined(adapter.contextSchema)) {
createContextWatcher(vm, wireDef, (newContext: ContextValue) => {
// every time the context is pushed into this component,
// this callback will be invoked with the new computed context
if (context !== newContext) {
context = newContext;
// Note: when new context arrives, the config will be recomputed and pushed along side the new
// context, this is to preserve the identity characteristics, config should not have identity
// (ever), while context can have identity
if (vm.state === VMState.connected) {
computeConfigAndUpdate();
}
}
});
}
contextWatcher = createContextWatcher(adapter, vm.elm, vm.renderer, computeConfigAndUpdate);

return {
// @ts-ignore the boundary protection executes sync, connector is always defined
connector,
computeConfigAndUpdate,
resetConfigWatcher: () => ro.reset(),
connect() {
runWithBoundaryProtection(
vm,
vm,
noop,
() => {
connector.connect();

contextWatcher?.connect();
},
noop
);

// computeConfigAndUpdate already has boundary protection
if (hasDynamicParams) {
Promise.resolve().then(computeConfigAndUpdate);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should probably have this as a utility method somewhere.

} else {
computeConfigAndUpdate();
}
},
disconnect() {
runWithBoundaryProtection(
vm,
vm,
noop,
() => {
connector.disconnect();

contextWatcher?.disconnect();
},
noop
);

ro.reset();
},
};
}

Expand Down Expand Up @@ -265,6 +303,12 @@ export function setAdapterToken(adapter: WireAdapterConstructor, token: string)
export type ContextValue = Record<string, any>;
export type ConfigCallback = (component: ComponentInterface) => ConfigValue;

interface ContextWatcher {
jodarove marked this conversation as resolved.
Show resolved Hide resolved
connect: () => void;
disconnect: () => void;
readonly contextValue: ContextValue | undefined;
}

export interface WireAdapterConstructor {
new (callback: DataCallback): WireAdapter;
configSchema?: Record<string, WireAdapterSchemaValue>;
Expand Down Expand Up @@ -315,8 +359,7 @@ export function installWireAdapters(vm: VM) {
def: { wire },
} = vm;

const wiredConnecting = (context.wiredConnecting = []);
const wiredDisconnecting = (context.wiredDisconnecting = []);
const wiredConnectors: WireConnector[] = (context.wiredConnectors = []);
jodarove marked this conversation as resolved.
Show resolved Hide resolved

for (const fieldNameOrMethod in wire) {
const descriptor = wire[fieldNameOrMethod];
Expand All @@ -325,49 +368,25 @@ export function installWireAdapters(vm: VM) {
assert.invariant(wireDef, `Internal Error: invalid wire definition found.`);
}
if (!isUndefined(wireDef)) {
const { connector, computeConfigAndUpdate, resetConfigWatcher } = createConnector(
vm,
fieldNameOrMethod,
wireDef
);
const hasDynamicParams = wireDef.dynamic.length > 0;
ArrayPush.call(wiredConnecting, () => {
connector.connect();
if (hasDynamicParams) {
Promise.resolve().then(computeConfigAndUpdate);
} else {
computeConfigAndUpdate();
}
});
ArrayPush.call(wiredDisconnecting, () => {
connector.disconnect();
resetConfigWatcher();
});
const connector = createConnector(vm, fieldNameOrMethod, wireDef);

wiredConnectors.push(connector);
jodarove marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
const connector = createConnector(vm, fieldNameOrMethod, wireDef);
wiredConnectors.push(connector);
wiredConnectors.push(createConnector(vm, fieldNameOrMethod, wireDef));

}
}
}

export function connectWireAdapters(vm: VM) {
const { wiredConnecting } = vm.context;
const { wiredConnectors } = vm.context;

for (let i = 0, len = wiredConnecting.length; i < len; i += 1) {
wiredConnecting[i]();
for (let i = 0, len = wiredConnectors.length; i < len; i += 1) {
wiredConnectors[i].connect();
}
}

export function disconnectWireAdapters(vm: VM) {
const { wiredDisconnecting } = vm.context;
const { wiredConnectors } = vm.context;

runWithBoundaryProtection(
vm,
vm,
noop,
() => {
// job
for (let i = 0, len = wiredDisconnecting.length; i < len; i += 1) {
wiredDisconnecting[i]();
}
},
noop
);
for (let i = 0, len = wiredConnectors.length; i < len; i += 1) {
wiredConnectors[i].disconnect();
}
}