-
Notifications
You must be signed in to change notification settings - Fork 392
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
Changes from all commits
98c2ff2
26328f2
56dc6a0
d47d7e2
084a707
1e3fcde
ff4c4cf
679b1bb
57dd9d5
16d9028
9ae57b0
cb3d306
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
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) { | ||
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(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
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
Yes, this is a goal – it should copy e.g. a
I'm using a single constructable stylesheet which is copied into every shadow root. Whenever we call There was a problem hiding this comment. Choose a reason for hiding this commentThe 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); | ||
} |
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() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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: There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, this is due to |
||
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 () => { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 {} |
There was a problem hiding this comment.
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.There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
9ae57b0