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

fix(engine-dom): refactor stylesheet API for consistency #2827

Merged
merged 8 commits into from
May 11, 2022
Merged

fix(engine-dom): refactor stylesheet API for consistency #2827

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
f624b08
fix(engine-dom): refactor stylesheet API for consistency
nolanlawson Apr 18, 2022
5b96076
fix: remove useless code comment
nolanlawson May 9, 2022
09a56ee
test: remove unnecessary test
nolanlawson May 9, 2022
1cad8bb
test: remove unnecessary test
nolanlawson May 9, 2022
6f69244
refactor: slight refactor
nolanlawson May 9, 2022
c127a0c
fix: add code comments
nolanlawson May 10, 2022
3c63e5f
fix: add code comments
nolanlawson May 10, 2022
165663a
fix: add better comment
nolanlawson May 10, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions packages/@lwc/engine-core/src/framework/hydration.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -204,6 +204,7 @@ function hydrateCustomElement(elm: Node, vnode: VCustomElement): Node | null {
mode,
owner,
tagName: sel,
hydrated: true,
});

vnode.elm = elm;
Expand Down
3 changes: 1 addition & 2 deletions packages/@lwc/engine-core/src/framework/main.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,8 +80,6 @@ export {
setGetProperty,
setHTMLElement,
setInsert,
setInsertGlobalStylesheet,
setInsertStylesheet,
setIsConnected,
setIsHydrating,
setIsNativeShadowDefined,
Expand All @@ -98,4 +96,5 @@ export {
setSetText,
setSsr,
setAddEventListener,
setInsertStylesheet,
} from '../renderer';
24 changes: 6 additions & 18 deletions packages/@lwc/engine-core/src/framework/stylesheet.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,7 @@
*/
import { ArrayJoin, ArrayPush, isArray, isNull, isUndefined, KEY__SCOPED_CSS } from '@lwc/shared';

import {
getClassList,
removeAttribute,
setAttribute,
insertGlobalStylesheet,
ssr,
isHydrating,
insertStylesheet,
} from '../renderer';
import { getClassList, removeAttribute, setAttribute, ssr, insertStylesheet } from '../renderer';

import api from './api';
import { RenderMode, ShadowMode, VM } from './vm';
Expand Down Expand Up @@ -207,9 +199,9 @@ export function createStylesheet(vm: VM, stylesheets: string[]): VNode | null {
const { renderMode, shadowMode } = vm;
if (renderMode === RenderMode.Shadow && shadowMode === ShadowMode.Synthetic) {
for (let i = 0; i < stylesheets.length; i++) {
insertGlobalStylesheet(stylesheets[i]);
insertStylesheet(stylesheets[i]);
}
} else if (ssr || isHydrating()) {
} else if (ssr || vm.hydrated) {
Copy link
Collaborator Author

@nolanlawson nolanlawson May 9, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

One downside of this is that it will slightly bloat the bundle size for environments without hydration. In the previous version of the code, if the hydrateComponent API is unused, then this becomes false and the entire else if block is tree-shaken, including the createInlineStyleVNode function.

In practice, this is like ~5 lines of code, so I don't think it's a big deal. Also, a sufficiently smart minifier could understand that vm.hydrated is always false, so we could potentially fix it in the future using a refactor. Or we could introduce a hydratable option like Svelte.

// Note: We need to ensure that during hydration, the stylesheets method is the same as those in ssr.
// This works in the client, because the stylesheets are created, and cached in the VM
// the first time the VM renders.
Expand All @@ -220,14 +212,10 @@ export function createStylesheet(vm: VM, stylesheets: string[]): VNode | null {
} else {
// native shadow or light DOM, DOM renderer
const root = getNearestNativeShadowComponent(vm);
const isGlobal = isNull(root);
// null root means a global style
const target = isNull(root) ? undefined : root.shadowRoot!;
for (let i = 0; i < stylesheets.length; i++) {
if (isGlobal) {
insertGlobalStylesheet(stylesheets[i]);
} else {
// local level
insertStylesheet(stylesheets[i], root!.shadowRoot!);
}
insertStylesheet(stylesheets[i], target);
}
}
return null;
Expand Down
6 changes: 5 additions & 1 deletion packages/@lwc/engine-core/src/framework/vm.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,8 @@ export interface VM<N = HostNode, E = HostElement> {
readonly context: Context;
/** The owner VM or null for root elements. */
readonly owner: VM<N, E> | null;
/** Whether or not the VM was hydrated */
readonly hydrated: boolean;
/** Rendering operations associated with the VM */
readonly renderMode: RenderMode;
shadowMode: ShadowMode;
Expand Down Expand Up @@ -263,9 +265,10 @@ export function createVM<HostNode, HostElement>(
mode: ShadowRootMode;
owner: VM<HostNode, HostElement> | null;
tagName: string;
hydrated?: boolean;
}
): VM {
const { mode, owner, tagName } = options;
const { mode, owner, tagName, hydrated } = options;
const def = getComponentInternalDef(ctor);

const vm: VM = {
Expand All @@ -286,6 +289,7 @@ export function createVM<HostNode, HostElement>(
cmpSlots: create(null),
oar: create(null),
cmpTemplate: null,
hydrated: Boolean(hydrated),

renderMode: def.renderMode,
shadowMode: computeShadowMode(def, owner),
Expand Down
9 changes: 1 addition & 8 deletions packages/@lwc/engine-core/src/renderer.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -245,14 +245,7 @@ export let isConnected: isConnectedFunc;
export function setIsConnected(isConnectedImpl: isConnectedFunc) {
isConnected = isConnectedImpl;
}

type insertGlobalStylesheetFunc = (content: string) => void;
export let insertGlobalStylesheet: insertGlobalStylesheetFunc;
export function setInsertGlobalStylesheet(insertGlobalStylesheetImpl: insertGlobalStylesheetFunc) {
insertGlobalStylesheet = insertGlobalStylesheetImpl;
}

type insertStylesheetFunc = (content: string, target: ShadowRoot) => void;
type insertStylesheetFunc = (content: string, target?: ShadowRoot) => void;
export let insertStylesheet: insertStylesheetFunc;
export function setInsertStylesheet(insertStylesheetImpl: insertStylesheetFunc) {
insertStylesheet = insertStylesheetImpl;
Expand Down
1 change: 1 addition & 0 deletions packages/@lwc/engine-dom/src/apis/hydrate-component.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ function createVMWithProps(element: Element, Ctor: typeof LightningElement, prop
mode: 'open',
owner: null,
tagName: element.tagName.toLowerCase(),
hydrated: true,
});

for (const [key, value] of Object.entries(props)) {
Expand Down
9 changes: 3 additions & 6 deletions packages/@lwc/engine-dom/src/initializeRenderer.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,6 @@ import {
setGetProperty,
setHTMLElement,
setInsert,
setInsertGlobalStylesheet,
setInsertStylesheet,
setIsConnected,
setIsHydrating,
setIsNativeShadowDefined,
Expand All @@ -46,6 +44,7 @@ import {
setSetText,
setSsr,
setAddEventListener,
setInsertStylesheet,
} from '@lwc/engine-core';

import {
Expand All @@ -71,8 +70,6 @@ import {
getProperty,
HTMLElement,
insert,
insertGlobalStylesheet,
insertStylesheet,
isConnected,
isHydrating,
isNativeShadowDefined,
Expand All @@ -89,6 +86,7 @@ import {
setText,
ssr,
addEventListener,
insertStylesheet,
} from './renderer';

setAssertInstanceOfHTMLElement(assertInstanceOfHTMLElement);
Expand All @@ -113,8 +111,6 @@ setGetLastElementChild(getLastElementChild);
setGetProperty(getProperty);
setHTMLElement(HTMLElement);
setInsert(insert);
setInsertGlobalStylesheet(insertGlobalStylesheet);
setInsertStylesheet(insertStylesheet);
setIsConnected(isConnected);
setIsHydrating(isHydrating);
setIsNativeShadowDefined(isNativeShadowDefined);
Expand All @@ -131,3 +127,4 @@ setSetProperty(setProperty);
setSetText(setText);
setSsr(ssr);
setAddEventListener(addEventListener);
setInsertStylesheet(insertStylesheet);
98 changes: 1 addition & 97 deletions packages/@lwc/engine-dom/src/renderer.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,39 +11,13 @@ import {
hasOwnProperty,
htmlPropertyToAttribute,
globalThis,
isFunction,
isUndefined,
isArray,
KEY__IS_NATIVE_SHADOW_ROOT_DEFINED,
KEY__SHADOW_TOKEN,
setPrototypeOf,
StringToLowerCase,
getOwnPropertyDescriptor,
} from '@lwc/shared';

const globalStylesheets: { [content: string]: true } = create(null);

if (process.env.NODE_ENV === 'development') {
// @ts-ignore
window.__lwcResetGlobalStylesheets = () => {
for (const key of Object.keys(globalStylesheets)) {
delete globalStylesheets[key];
}
};
}

const globalStylesheetsParentElement: Element = document.head || document.body || document;
// This check for constructable stylesheets is similar to Fast's:
// https://github.com/microsoft/fast/blob/d49d1ec/packages/web-components/fast-element/src/dom.ts#L51-L53
// See also: https://github.com/whatwg/webidl/issues/1027#issuecomment-934510070
const supportsConstructableStyleSheets =
isFunction(CSSStyleSheet.prototype.replaceSync) && isArray(document.adoptedStyleSheets);
const supportsMutableAdoptedStyleSheets =
supportsConstructableStyleSheets &&
getOwnPropertyDescriptor(document.adoptedStyleSheets, 'length')!.writable;
const styleElements: { [content: string]: HTMLStyleElement } = create(null);
const styleSheets: { [content: string]: CSSStyleSheet } = create(null);
const shadowRootsToStyleSheets = new WeakMap<ShadowRoot, { [content: string]: true }>();
export { insertStylesheet } from './styles';

export let getCustomElement: any;
export let defineCustomElement: any;
Expand Down Expand Up @@ -72,53 +46,6 @@ function isCustomElementRegistryAvailable() {
}
}

function insertConstructableStyleSheet(content: string, target: ShadowRoot) {
// It's important for CSSStyleSheets to be unique based on their content, so that
// `shadowRoot.adoptedStyleSheets.includes(sheet)` works.
let styleSheet = styleSheets[content];
if (isUndefined(styleSheet)) {
styleSheet = new CSSStyleSheet();
styleSheet.replaceSync(content);
styleSheets[content] = styleSheet;
}
const { adoptedStyleSheets } = target;
if (!adoptedStyleSheets.includes(styleSheet)) {
if (supportsMutableAdoptedStyleSheets) {
// This is only supported in later versions of Chromium:
// https://chromestatus.com/feature/5638996492288000
adoptedStyleSheets.push(styleSheet);
} else {
target.adoptedStyleSheets = [...adoptedStyleSheets, styleSheet];
}
}
}

function insertStyleElement(content: string, target: ShadowRoot) {
// Avoid inserting duplicate `<style>`s
let sheets = shadowRootsToStyleSheets.get(target);
if (isUndefined(sheets)) {
sheets = create(null);
shadowRootsToStyleSheets.set(target, sheets!);
}
if (sheets![content]) {
return;
}
sheets![content] = true;

// This `<style>` may be repeated multiple times in the DOM, so cache it. It's a bit
// faster to call `cloneNode()` on an existing node than to recreate it every time.
let elm = styleElements[content];
if (isUndefined(elm)) {
elm = document.createElement('style');
elm.type = 'text/css';
elm.textContent = content;
styleElements[content] = elm;
} else {
elm = elm.cloneNode(true) as HTMLStyleElement;
}
target.appendChild(elm);
}

if (isCustomElementRegistryAvailable()) {
getCustomElement = customElements.get.bind(customElements);
defineCustomElement = customElements.define.bind(customElements);
Expand Down Expand Up @@ -349,29 +276,6 @@ export function isConnected(node: Node): boolean {
return node.isConnected;
}

export function insertGlobalStylesheet(content: string): void {
if (!isUndefined(globalStylesheets[content])) {
return;
}

globalStylesheets[content] = true;

const elm = document.createElement('style');
elm.type = 'text/css';
elm.textContent = content;

globalStylesheetsParentElement.appendChild(elm);
}

export function insertStylesheet(content: string, target: ShadowRoot): void {
if (supportsConstructableStyleSheets) {
insertConstructableStyleSheet(content, target);
} else {
// Fall back to <style> element
insertStyleElement(content, target);
}
}

export function assertInstanceOfHTMLElement(elm: any, msg: string) {
assert.invariant(elm instanceof HTMLElement, msg);
}
Expand Down
Loading