Skip to content

Commit

Permalink
chore: add flag for extended scope token syntax @W-17710895 (#5211)
Browse files Browse the repository at this point in the history
* chore: add flag for extended scope token syntax

* test(engine-server): remove `feature` feature

it's unused

* chore: remove feature from types

* chore(tests): remove unused exports from tests

* chore: rename x/test modules to x/static

* chore: use static x-test for root fixture component

tag name (x-test) may not match directory name of root component

* chore: remove tag name that is no longer used

* refactor(test): destructure!

* chore: `fixture-test` feels better than `x-test` to make it more clear it's special

* refactor: check scope token for static optimized on/off

* test: invalid scope tokens

* test: restore `features` feature

just removed it yesterday lol

* test(engine-server): add scope token validation tests

* chore: remove unused file

* chore: remove debuggery prop

* chore: update to new test config

* chore: prettier

* chore: rename flag

make it more accurate

* chore: restore original validation location

* chore: clean up duplicate check

* test(karma): update test to new behavior
  • Loading branch information
wjhsf authored Feb 28, 2025
1 parent 340e4a4 commit fa4c927
Show file tree
Hide file tree
Showing 23 changed files with 97 additions and 12 deletions.
10 changes: 9 additions & 1 deletion packages/@lwc/engine-core/src/framework/rendering.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ import { logError } from '../shared/logger';
import { getComponentTag } from '../shared/format';
import { EmptyArray, shouldBeFormAssociated } from './utils';
import { markComponentAsDirty } from './component';
import { getScopeTokenClass } from './stylesheet';
import { getScopeTokenClass, isValidScopeToken } from './stylesheet';
import { lockDomMutation, patchElementWithRestrictions, unlockDomMutation } from './restrictions';
import {
appendVM,
Expand Down Expand Up @@ -605,6 +605,10 @@ function applyStyleScoping(elm: Element, owner: VM, renderer: RendererAPI) {
// Set the class name for `*.scoped.css` style scoping.
const scopeToken = getScopeTokenClass(owner, /* legacy */ false);
if (!isNull(scopeToken)) {
if (!isValidScopeToken(scopeToken)) {
// See W-16614556
throw new Error('stylesheet token must be a valid string');
}
// TODO [#2762]: this dot notation with add is probably problematic
// probably we should have a renderer api for just the add operation
getClassList(elm).add(scopeToken);
Expand All @@ -614,6 +618,10 @@ function applyStyleScoping(elm: Element, owner: VM, renderer: RendererAPI) {
if (lwcRuntimeFlags.ENABLE_LEGACY_SCOPE_TOKENS) {
const legacyScopeToken = getScopeTokenClass(owner, /* legacy */ true);
if (!isNull(legacyScopeToken)) {
if (!isValidScopeToken(legacyScopeToken)) {
// See W-16614556
throw new Error('stylesheet token must be a valid string');
}
// TODO [#2762]: this dot notation with add is probably problematic
// probably we should have a renderer api for just the add operation
getClassList(elm).add(legacyScopeToken);
Expand Down
12 changes: 12 additions & 0 deletions packages/@lwc/engine-core/src/framework/stylesheet.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import {
ArrayPush,
isArray,
isNull,
isString,
isTrue,
isUndefined,
KEY__NATIVE_ONLY_CSS,
Expand All @@ -29,6 +30,8 @@ import type { Template } from './template';
import type { VM } from './vm';
import type { Stylesheet, Stylesheets } from '@lwc/shared';

const VALID_SCOPE_TOKEN_REGEX = /^[a-zA-Z0-9\-_]+$/;

// These are only used for HMR in dev mode
// The "pure" annotations are so that Rollup knows for sure it can remove these from prod mode
let stylesheetsToCssContent: WeakMap<Stylesheet, Set<string>> = /*@__PURE__@*/ new WeakMap();
Expand Down Expand Up @@ -394,3 +397,12 @@ export function unrenderStylesheet(stylesheet: Stylesheet) {
cssContentToAbortControllers.delete(cssContent);
}
}

export function isValidScopeToken(token: unknown) {
if (!isString(token)) {
return false;
}

// See W-16614556
return lwcRuntimeFlags.DISABLE_SCOPE_TOKEN_VALIDATION || VALID_SCOPE_TOKEN_REGEX.test(token);
}
16 changes: 7 additions & 9 deletions packages/@lwc/engine-core/src/framework/template.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,12 @@ import api from './api';
import { RenderMode, resetComponentRoot, runWithBoundaryProtection, ShadowMode } from './vm';
import { assertNotProd, EmptyObject } from './utils';
import { defaultEmptyTemplate, isTemplateRegistered } from './secure-template';
import { createStylesheet, getStylesheetsContent, updateStylesheetToken } from './stylesheet';
import {
createStylesheet,
getStylesheetsContent,
isValidScopeToken,
updateStylesheetToken,
} from './stylesheet';
import { logOperationEnd, logOperationStart, OperationId } from './profiler';
import { getTemplateOrSwappedTemplate, setActiveVM } from './hot-swaps';
import { getMapFromClassName } from './modules/computed-class-attr';
Expand Down Expand Up @@ -66,14 +71,6 @@ export function setVMBeingRendered(vm: VM | null) {
vmBeingRendered = vm;
}

const VALID_SCOPE_TOKEN_REGEX = /^[a-zA-Z0-9\-_]+$/;

// See W-16614556
// TODO [#2826]: freeze the template object
function isValidScopeToken(token: any) {
return isString(token) && VALID_SCOPE_TOKEN_REGEX.test(token);
}

function validateSlots(vm: VM) {
assertNotProd(); // this method should never leak to prod

Expand Down Expand Up @@ -274,6 +271,7 @@ function buildParseFragmentFn(
}

// See W-16614556
// TODO [#2826]: freeze the template object
if (
(hasStyleToken && !isValidScopeToken(stylesheetToken)) ||
(hasLegacyToken && !isValidScopeToken(legacyStylesheetToken))
Expand Down
9 changes: 9 additions & 0 deletions packages/@lwc/engine-server/src/__tests__/fixtures.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -132,8 +132,13 @@ function testFixtures(options?: RollupLwcOptions) {
let result;
let err;
try {
config?.features?.forEach((flag) => {
lwcEngineServer.setFeatureFlagForTest(flag, true);
});

const module: LightningElementConstructor = (await import(compiledFixturePath))
.default;

result = formatHTML(
lwcEngineServer.renderComponent('fixture-test', module, config?.props ?? {})
);
Expand All @@ -144,6 +149,10 @@ function testFixtures(options?: RollupLwcOptions) {
err = _err?.message || 'An empty error occurred?!';
}

config?.features?.forEach((flag) => {
lwcEngineServer.setFeatureFlagForTest(flag, false);
});

return {
'expected.html': result,
'error.txt': err,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
{
"entry": "x/component",
"features": ["DISABLE_SCOPE_TOKEN_VALIDATION"]
}
Empty file.
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
<fixture-test class="stylesheet.token-host" data-lwc-host-scope-token="stylesheet.token-host">
<style class="stylesheet.token" type="text/css">
p.stylesheet.token {font-size: 2em;}
</style>
<p class="stylesheet.token">
je suis une pomme de terre
</p>
</fixture-test>
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
<template lwc:render-mode="light">
<p>je suis une pomme de terre</p>
</template>
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
import { LightningElement } from 'lwc';
import cmp from './component.html';
cmp.stylesheetToken = 'stylesheet.token';

export default class HelloWorld extends LightningElement {
static renderMode = 'light';
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
p {
font-size: 2em;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
<template lwc:render-mode="light">
<p>je suis une pomme de terre</p>
</template>
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"entry": "x/component"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
stylesheet token must be a valid string
Empty file.
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
<template lwc:render-mode="light">
<p>je suis une pomme de terre</p>
</template>
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
import { LightningElement } from 'lwc';
import cmp from './component.html';
cmp.stylesheetToken = 'stylesheet.token';

export default class HelloWorld extends LightningElement {
static renderMode = 'light';
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
p {
font-size: 2em;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
<template lwc:render-mode="light">
<p>je suis une pomme de terre</p>
</template>
1 change: 1 addition & 0 deletions packages/@lwc/features/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ const features: FeatureFlagMap = {
ENABLE_FORCE_SHADOW_MIGRATE_MODE: null,
ENABLE_EXPERIMENTAL_SIGNALS: null,
DISABLE_SYNTHETIC_SHADOW: null,
DISABLE_SCOPE_TOKEN_VALIDATION: null,
LEGACY_LOCKER_ENABLED: null,
};

Expand Down
5 changes: 5 additions & 0 deletions packages/@lwc/features/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,11 @@ export interface FeatureFlagMap {
*/
DISABLE_SYNTHETIC_SHADOW: FeatureFlagValue;

/**
* If true, the contents of stylesheet scope tokens are not validated.
*/
DISABLE_SCOPE_TOKEN_VALIDATION: FeatureFlagValue;

/**
* If true, then lightning legacy locker is supported, otherwise lightning legacy locker will not function
* properly.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,10 +67,11 @@ props.forEach((prop) => {

if (
process.env.NATIVE_SHADOW &&
process.env.DISABLE_STATIC_CONTENT_OPTIMIZATION
process.env.DISABLE_STATIC_CONTENT_OPTIMIZATION &&
Ctor !== Scoping
) {
// If we're rendering in native shadow and the static content optimization is disabled,
// then there's no problem with non-string stylesheet tokens because they are only rendered
// then there's no problem with invalid stylesheet tokens because they are only rendered
// as class attribute values using either `classList` or `setAttribute` (and this only applies
// when `*.scoped.css` is being used).
expect(elm.shadowRoot.children.length).toBe(1);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,4 +12,6 @@ export const expectedFailures = new Set([
'attribute-global-html/as-component-prop/without-@api/config.json',
'known-boolean-attributes/default-def-html-attributes/static-on-component/config.json',
'wire/errors/throws-when-colliding-prop-then-method/config.json',
'scope-token/config.json',
'scope-token-extended/config.json',
]);
1 change: 1 addition & 0 deletions scripts/test-utils/test-fixture-dir.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import path from 'node:path';
import { AssertionError } from 'node:assert';
import { test } from 'vitest';
import * as glob from 'glob';

const { globSync } = glob;

type TestFixtureOutput = { [filename: string]: unknown };
Expand Down

0 comments on commit fa4c927

Please sign in to comment.