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

feat: add flag for forced shadow migrate mode #3894

Merged
merged 12 commits into from
Dec 11, 2023
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,9 @@ import {
freeze,
hasOwnProperty,
isFunction,
isString,
isNull,
isObject,
isString,
isUndefined,
KEY__SYNTHETIC_MODE,
keys,
Expand All @@ -35,7 +35,7 @@ import { applyAriaReflection } from '../libs/aria-reflection/aria-reflection';

import { HTMLElementOriginalDescriptors } from './html-properties';
import { getWrappedComponentsListener } from './component';
import { vmBeingConstructed, isBeingConstructed, isInvokingRender } from './invoker';
import { isBeingConstructed, isInvokingRender, vmBeingConstructed } from './invoker';
import {
associateVM,
getAssociatedVM,
Expand All @@ -47,16 +47,17 @@ import {
} from './vm';
import { componentValueObserved } from './mutation-tracker';
import {
patchShadowRootWithRestrictions,
patchLightningElementPrototypeWithRestrictions,
patchCustomElementWithRestrictions,
patchLightningElementPrototypeWithRestrictions,
patchShadowRootWithRestrictions,
} from './restrictions';
import { Template, isUpdatingTemplate, getVMBeingRendered } from './template';
import { getVMBeingRendered, isUpdatingTemplate, Template } from './template';
import { HTMLElementConstructor } from './base-bridge-element';
import { updateComponentValue } from './update-component-value';
import { markLockerLiveObject } from './membrane';
import { TemplateStylesheetFactories } from './stylesheet';
import { instrumentInstance } from './runtime-instrumentation';
import { applyShadowMigrateMode } from './shadow-migration-mode';

/**
* This operation is called with a descriptor of an standard html property
Expand Down Expand Up @@ -289,6 +290,14 @@ function doAttachShadow(vm: VM): ShadowRoot {
patchShadowRootWithRestrictions(shadowRoot);
}

if (
process.env.IS_BROWSER &&
lwcRuntimeFlags.ENABLE_FORCE_SHADOW_MIGRATE_MODE &&
vm.shadowMigrateMode
) {
applyShadowMigrateMode(shadowRoot);
}

return shadowRoot;
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
/*
* Copyright (c) 2023, salesforce.com, inc.
* All rights reserved.
* SPDX-License-Identifier: MIT
* For full license text, see the LICENSE file in the repo root or https://opensource.org/licenses/MIT
*/
import { logWarnOnce } from '../shared/logger';

let globalStylesheet: CSSStyleSheet | undefined;

function isStyleElement(elm: Element): elm is HTMLStyleElement {
return elm.tagName === 'STYLE';
}

async function fetchStylesheet(elm: HTMLStyleElement | HTMLLinkElement) {
if (isStyleElement(elm)) {
return elm.textContent;
} else {
// <link>
const { href } = elm;
try {
return await (await fetch(href)).text();
} catch (err) {
logWarnOnce(`Ignoring cross-origin stylesheet in migrate mode: ${href}`);
// ignore errors with cross-origin stylesheets - nothing we can do for those
return '';
}
}
}

function initGlobalStylesheet() {
const stylesheet = new CSSStyleSheet();
const elmsToPromises = new Map();
let lastSeenLength = 0;

const copyToGlobalStylesheet = () => {
const elms = document.head.querySelectorAll(
'style:not([data-rendered-by-lwc]),link[rel="stylesheet"]'
);
if (elms.length === lastSeenLength) {
Copy link
Member

@jmsjtu jmsjtu Dec 11, 2023

Choose a reason for hiding this comment

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

I'm not sure if this would happen or if it's a concern but if someone updates an element for example:

<link rel=stylesheet src=app.css> -> <link rel=stylesheet src=app2.css>

The length of the elms would be the same and the update would be skipped.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yep, good point! I doubt we will need to handle that case, though. I think in most cases people are just adding/removing link tags. But I can add a comment about this.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

return; // nothing to update
}
lastSeenLength = elms.length;
const promises = [...(elms as unknown as Iterable<HTMLStyleElement | HTMLLinkElement>)].map(
(elm) => {
let promise = elmsToPromises.get(elm);
if (!promise) {
// Cache the promise
promise = fetchStylesheet(elm);
elmsToPromises.set(elm, promise);
}
return promise;
}
);
Promise.all(promises).then((stylesheetTexts) => {
// When replaceSync() is called, the entire contents of the constructable stylesheet are replaced
// with the copied+concatenated styles. This means that any shadow root's adoptedStyleSheets that
// contains this constructable stylesheet will immediately get the new styles.
stylesheet.replaceSync(stylesheetTexts.join('\n'));
});
};

const headObserver = new MutationObserver(copyToGlobalStylesheet);

// By observing only the childList, note that we are not covering the case where someone changes an `href`
// on an existing <link>`, or the textContent on an existing `<style>`. This is assumed to be an uncommon
// case and not worth covering.
headObserver.observe(document.head, {
childList: true,
});

copyToGlobalStylesheet();

return stylesheet;
}

export function applyShadowMigrateMode(shadowRoot: ShadowRoot) {
if (!globalStylesheet) {
globalStylesheet = initGlobalStylesheet();
Copy link
Contributor

Choose a reason for hiding this comment

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

This does not have to be eager because, the styles are not needed until the first LWC element is ready to use it. And the mutation observer's callback queries for all stylesheets not added by lwc and inserts them. So stylesheets added before the MO was initialized will also be copied over.

Question: What if the stylesheets are added to the document.head after doAttachShadow, these don't get copied over until the next time the component is reinstantiated. If it gets rehydrated, it still won't get the styles. Correct?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This does not have to be eager because, the styles are not needed until the first LWC element is ready to use it.

This is true, but it seems to me it would complicate the code to wait until the precise moment an LWC component needs the styles, which is after renderedCallback. Here we are doing it at connectedCallback (which is when attachShadow is called), which is right before renderedCallback, so there's really not much difference.

So stylesheets added before the MO was initialized will also be copied over.

Yes, this is a goal – it should copy e.g. a <link rel=stylesheet src=app.css> that already existed in the HTML even before JS ran.

What if the stylesheets are added to the document.head after doAttachShadow, these don't get copied over until the next time the component is reinstantiated. If it gets rehydrated, it still won't get the styles. Correct?

I'm using a single constructable stylesheet which is copied into every shadow root. Whenever we call replaceSync, it automatically updates everywhere that that constructable stylesheet is used. I can add a comment about this.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

}

(shadowRoot as any).synthetic = true; // pretend to be synthetic mode
shadowRoot.adoptedStyleSheets.push(globalStylesheet);
}
14 changes: 12 additions & 2 deletions packages/@lwc/engine-core/src/framework/vm.ts
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,8 @@ export interface VM<N = HostNode, E = HostElement> {
/** Rendering operations associated with the VM */
renderMode: RenderMode;
shadowMode: ShadowMode;
/** True if shadow migrate mode is in effect, i.e. this is native with synthetic-like modifications */
shadowMigrateMode: boolean;
/** The component creation index. */
idx: number;
/** Component state, analogous to Element.isConnected */
Expand Down Expand Up @@ -353,6 +355,7 @@ export function createVM<HostNode, HostElement>(
// Properties set right after VM creation.
tro: null!,
shadowMode: null!,
shadowMigrateMode: false,
stylesheets: null!,

// Properties set by the LightningElement constructor.
Expand All @@ -373,7 +376,13 @@ export function createVM<HostNode, HostElement>(
}

vm.stylesheets = computeStylesheets(vm, def.ctor);
vm.shadowMode = computeShadowMode(def, vm.owner, renderer);
const computedShadowMode = computeShadowMode(def, vm.owner, renderer);
if (lwcRuntimeFlags.ENABLE_FORCE_SHADOW_MIGRATE_MODE) {
vm.shadowMode = ShadowMode.Native;
vm.shadowMigrateMode = computedShadowMode === ShadowMode.Synthetic;
} else {
vm.shadowMode = computedShadowMode;
}
vm.tro = getTemplateReactiveObserver(vm);

// We don't need to report the shadow mode if we're rendering in light DOM
Expand Down Expand Up @@ -490,7 +499,8 @@ function computeShadowMode(def: ComponentDef, owner: VM | null, renderer: Render
const { isSyntheticShadowDefined } = renderer;

let shadowMode;
if (isSyntheticShadowDefined) {
// If ENABLE_FORCE_SHADOW_MIGRATE_MODE is true, then ShadowMode.Synthetic here will mean "force-migrate" mode.
if (isSyntheticShadowDefined || lwcRuntimeFlags.ENABLE_FORCE_SHADOW_MIGRATE_MODE) {
if (def.renderMode === RenderMode.Light) {
// ShadowMode.Native implies "not synthetic shadow" which is consistent with how
// everything defaults to native when the synthetic shadow polyfill is unavailable.
Expand Down
2 changes: 2 additions & 0 deletions packages/@lwc/engine-dom/src/styles.ts
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,8 @@ function createFreshStyleElement(content: string) {
const elm = document.createElement('style');
elm.type = 'text/css';
elm.textContent = content;
// Add an attribute to distinguish global styles added by LWC as opposed to other frameworks/libraries on the page
elm.setAttribute('data-rendered-by-lwc', '');
nolanlawson marked this conversation as resolved.
Show resolved Hide resolved
return elm;
}

Expand Down
1 change: 1 addition & 0 deletions packages/@lwc/features/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ const features: FeatureFlagMap = {
DISABLE_LIGHT_DOM_UNSCOPED_CSS: null,
ENABLE_FROZEN_TEMPLATE: null,
ENABLE_LEGACY_SCOPE_TOKENS: null,
ENABLE_FORCE_SHADOW_MIGRATE_MODE: null,
};

// eslint-disable-next-line no-restricted-properties
Expand Down
4 changes: 4 additions & 0 deletions packages/@lwc/features/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,10 @@ export interface FeatureFlagMap {
*/
// TODO [#3733]: remove support for legacy scope tokens
ENABLE_LEGACY_SCOPE_TOKENS: FeatureFlagValue;
/**
* If true, enable experimental shadow DOM migration mode globally.
*/
ENABLE_FORCE_SHADOW_MIGRATE_MODE: FeatureFlagValue;
}

export type FeatureFlagName = keyof FeatureFlagMap;
Original file line number Diff line number Diff line change
@@ -0,0 +1,146 @@
import { createElement, setFeatureFlagForTest } from 'lwc';
import Native from 'x/native';
import Synthetic from 'x/synthetic';
import StyledLight from 'x/styledLight';

function doubleMicrotask() {
// wait for applyShadowMigrateMode()
Copy link
Contributor

Choose a reason for hiding this comment

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

The double wait is due to the usage of MutationObserver? I ask because the lwc:dom tests work with one microtask: packages/@lwc/integration-karma/test/dom-manual/moving-node-to-document/index.spec.js

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, this is due to applyShadowMigrateMode. TBH I didn't look super deep into why it's two microtasks rather than just one.

return Promise.resolve().then(() => Promise.resolve());
}

function isActuallyNativeShadow(shadowRoot) {
return shadowRoot.constructor.toString().includes('[native code]');
}

function isClaimingToBeSyntheticShadow(shadowRoot) {
return !!shadowRoot.synthetic;
}

// This test only makes sense for true native shadow mode, because if ENABLE_FORCE_SHADOW_MIGRATE_MODE is true,
// then the polyfill should not be loaded at all.
if (process.env.NATIVE_SHADOW && !process.env.MIXED_SHADOW) {
describe('shadow migrate mode', () => {
beforeEach(async () => {
const style = document.createElement('style');
style.textContent = 'h1 { color: blue }';
document.head.appendChild(style);
});

describe('flag on', () => {
beforeEach(() => {
setFeatureFlagForTest('ENABLE_FORCE_SHADOW_MIGRATE_MODE', true);
});

afterEach(() => {
setFeatureFlagForTest('ENABLE_FORCE_SHADOW_MIGRATE_MODE', false);
});

it('uses global styles for synthetic components', async () => {
const elm = createElement('x-synthetic', { is: Synthetic });
document.body.appendChild(elm);

await doubleMicrotask();

expect(isActuallyNativeShadow(elm.shadowRoot)).toBe(true);
expect(isClaimingToBeSyntheticShadow(elm.shadowRoot)).toBe(true);
expect(getComputedStyle(elm.shadowRoot.querySelector('h1')).color).toBe(
'rgb(0, 0, 255)'
);
});

it('does not use global styles for native components', async () => {
const elm = createElement('x-native', { is: Native });
document.body.appendChild(elm);

await doubleMicrotask();

expect(isActuallyNativeShadow(elm.shadowRoot)).toBe(true);
expect(isClaimingToBeSyntheticShadow(elm.shadowRoot)).toBe(false);
expect(getComputedStyle(elm.shadowRoot.querySelector('h1')).color).toBe(
'rgb(0, 0, 0)'
);
});

it('does not apply styles from global light DOM components to synthetic components', async () => {
const light = createElement('x-styled-light', { is: StyledLight });
document.body.appendChild(light);

const elm = createElement('x-synthetic', { is: Synthetic });
document.body.appendChild(elm);

await doubleMicrotask();

expect(getComputedStyle(elm.shadowRoot.querySelector('h1')).opacity).toBe('1');
expect(getComputedStyle(light.querySelector('h1')).opacity).toBe('0.5');
});

it('uses new styles added to the head after component is rendered', async () => {
const elm = createElement('x-synthetic', { is: Synthetic });
document.body.appendChild(elm);

await doubleMicrotask();

expect(getComputedStyle(elm.shadowRoot.querySelector('h1')).color).toBe(
'rgb(0, 0, 255)'
);

const style = document.createElement('style');
style.textContent = `h1 { color: purple; background-color: crimson }`;
document.head.appendChild(style);

await doubleMicrotask();

expect(getComputedStyle(elm.shadowRoot.querySelector('h1')).color).toBe(
'rgb(128, 0, 128)'
);
expect(getComputedStyle(elm.shadowRoot.querySelector('h1')).backgroundColor).toBe(
'rgb(220, 20, 60)'
);
});

it('local styles are defined after global styles', async () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

👌

const elm = createElement('x-synthetic', { is: Synthetic });
document.body.appendChild(elm);

const style = document.createElement('style');
style.textContent = `h1 { font-family: sans-serif; background-color: green; }`;
document.head.appendChild(style);

await doubleMicrotask();

expect(getComputedStyle(elm.shadowRoot.querySelector('h1')).backgroundColor).toBe(
'rgb(0, 128, 0)'
);
expect(getComputedStyle(elm.shadowRoot.querySelector('h1')).fontFamily).toBe(
'monospace'
);
});
});

describe('flag off', () => {
it('does not use global styles for synthetic components', async () => {
const elm = createElement('x-synthetic', { is: Synthetic });
document.body.appendChild(elm);

await doubleMicrotask();
expect(isActuallyNativeShadow(elm.shadowRoot)).toBe(true);
expect(isClaimingToBeSyntheticShadow(elm.shadowRoot)).toBe(false);
expect(getComputedStyle(elm.shadowRoot.querySelector('h1')).color).toBe(
'rgb(0, 0, 0)'
);
});

it('does not use global styles for native components', async () => {
const elm = createElement('x-native', { is: Native });
document.body.appendChild(elm);

await doubleMicrotask();
expect(isActuallyNativeShadow(elm.shadowRoot)).toBe(true);
expect(isClaimingToBeSyntheticShadow(elm.shadowRoot)).toBe(false);
expect(getComputedStyle(elm.shadowRoot.querySelector('h1')).color).toBe(
'rgb(0, 0, 0)'
);
});
});
});
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
<template>
<h1>Hello</h1>
</template>
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
import { LightningElement } from 'lwc';

export default class extends LightningElement {
static shadowSupportMode = 'native';
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
h1 {
opacity: 0.5;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
<template lwc:render-mode="light">
<h1>Hello</h1>
</template>
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
import { LightningElement } from 'lwc';

export default class extends LightningElement {
static renderMode = 'light';
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
h1 {
font-family: monospace;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
<template>
<h1>Hello</h1>
</template>
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
import { LightningElement } from 'lwc';

export default class extends LightningElement {}
Loading