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

fix: implement stylesheet registration and disallow arbitrary functions #3477

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
* For full license text, see the LICENSE file in the repo root or https://opensource.org/licenses/MIT
*/
import { TransformOptions } from '../../options';
import { transform } from '../transformer';
import { transform, transformSync } from '../transformer';

const TRANSFORMATION_OPTIONS: TransformOptions = {
namespace: 'x',
Expand All @@ -21,6 +21,7 @@ it('should throw when processing an invalid CSS file', async () => {

it('should apply transformation for stylesheet file', async () => {
const actual = `
@import 'x/sibling';
:host {
color: red;
}
Expand All @@ -30,10 +31,28 @@ it('should apply transformation for stylesheet file', async () => {
}
`;
const { code } = await transform(actual, 'foo.css', TRANSFORMATION_OPTIONS);

expect(code).toContain('function stylesheet');
});

it('should import registerStylesheet and register only the generated stylesheet', () => {
const actual = `
@import 'x/sibling';
:host {
color: red;
}

div {
background-color: red;
}
`;

const { code } = transformSync(actual, 'foo.css', TRANSFORMATION_OPTIONS);
expect(code).toContain('import { registerStylesheet }');

// Verify only the single generated stylesheet is registered (no imported stylesheets)
expect(code).toContain('registerStylesheet(stylesheet);');
});

describe('custom properties', () => {
it('should not transform var functions if custom properties a resolved natively', async () => {
const actual = `div { color: var(--bg-color); }`;
Expand Down
1 change: 1 addition & 0 deletions packages/@lwc/engine-core/src/framework/main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ export { hydrateRoot } from './hydration';
export { registerComponent } from './component';
export { registerTemplate } from './secure-template';
export { registerDecorators } from './decorators/register';
export { registerStylesheet } from './stylesheet';

// Mics. internal APIs -----------------------------------------------------------------------------
export { unwrap } from './membrane';
Expand Down
2 changes: 2 additions & 0 deletions packages/@lwc/engine-core/src/framework/reporting.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ export const enum ReportingEventId {
NonStandardAriaReflection = 'NonStandardAriaReflection',
TemplateMutation = 'TemplateMutation',
StylesheetMutation = 'StylesheetMutation',
UnexpectedStylesheetContent = 'UnexpectedStylesheetContent',
}

export interface BasePayload {
Expand Down Expand Up @@ -47,6 +48,7 @@ export type ReportingPayloadMapping = {
[ReportingEventId.NonStandardAriaReflection]: NonStandardAriaReflectionPayload;
[ReportingEventId.TemplateMutation]: TemplateMutationPayload;
[ReportingEventId.StylesheetMutation]: StylesheetMutationPayload;
[ReportingEventId.UnexpectedStylesheetContent]: BasePayload;
};

export type ReportingDispatcher<T extends ReportingEventId = ReportingEventId> = (
Expand Down
31 changes: 30 additions & 1 deletion packages/@lwc/engine-core/src/framework/stylesheet.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
*/
import { ArrayMap, ArrayPush, isArray, isNull, isUndefined, KEY__SCOPED_CSS } from '@lwc/shared';

import { logError } from '../shared/logger';
import { logError, logWarnOnce } from '../shared/logger';

import api from './api';
import { RenderMode, ShadowMode, VM } from './vm';
Expand All @@ -15,6 +15,7 @@ import { getStyleOrSwappedStyle } from './hot-swaps';
import { VCustomElement, VNode } from './vnodes';
import { checkVersionMismatch } from './check-version-mismatch';
import { getComponentInternalDef } from './def';
import { report, ReportingEventId } from './reporting';

/**
* Function producing style based on a host and a shadow selector. This function is invoked by
Expand Down Expand Up @@ -132,6 +133,19 @@ function evaluateStylesheetsContent(
// the stylesheet, while internally, we have a replacement for it.
stylesheet = getStyleOrSwappedStyle(stylesheet);
}

// Check that this stylesheet was generated by our compiler
if (!isStylesheetRegistered(stylesheet)) {
if (process.env.NODE_ENV !== 'production') {
logWarnOnce(
`TypeError: Unexpected LWC stylesheet content found for component <${vm.tagName.toLowerCase()}>.`
);
}
report(ReportingEventId.UnexpectedStylesheetContent, {
tagName: vm.tagName.toLowerCase(),
});
}

const isScopedCss = (stylesheet as any)[KEY__SCOPED_CSS];

if (
Expand Down Expand Up @@ -275,3 +289,18 @@ export function createStylesheet(vm: VM, stylesheets: string[]): VNode[] | null
}
return null;
}

const signedStylesheetSet: Set<StylesheetFactory> = new Set();

/**
* INTERNAL: This function can only be invoked by compiled code. The compiler
* will prevent this function from being imported by userland code.
*/
export function registerStylesheet(stylesheet: StylesheetFactory): StylesheetFactory {
signedStylesheetSet.add(stylesheet);
return stylesheet;
}

function isStylesheetRegistered(stylesheet: StylesheetFactory): boolean {
return signedStylesheetSet.has(stylesheet);
}
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ function checkAndReportViolation(elm: Element, prop: string, isSetter: boolean,
if (process.env.NODE_ENV !== 'production') {
logWarnOnce(
`Element <${elm.tagName.toLowerCase()}> ` +
(isUndefined(vm) ? '' : `owned by <${vm.elm.tagName.toLowerCase()}> `) +
(isUndefined(vm) ? '' : `owned by <${vm.tagName.toLowerCase()}> `) +
`uses non-standard property "${prop}". This will be removed in a future version of LWC. ` +
`See https://sfdc.co/deprecated-aria`
);
Expand Down
1 change: 1 addition & 0 deletions packages/@lwc/engine-dom/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ export {
freezeTemplate,
registerComponent,
registerDecorators,
registerStylesheet,
sanitizeAttribute,
setHooks,
getComponentDef,
Expand Down
1 change: 1 addition & 0 deletions packages/@lwc/engine-server/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ export {
freezeTemplate,
registerComponent,
registerDecorators,
registerStylesheet,
sanitizeAttribute,
setHooks,
getComponentDef,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
import {
createElement,
LightningElement,
registerComponent,
registerStylesheet,
registerTemplate,
__unstable__ReportingControl as reportingControl,
} from 'lwc';

describe('registerStylesheet', () => {
let dispatcher;

beforeEach(() => {
dispatcher = jasmine.createSpy();
reportingControl.attachDispatcher(dispatcher);
});

afterEach(() => {
reportingControl.detachDispatcher();
});

it('should accept a function and return the same value', () => {
const stylesheet = () => '';
const result = registerStylesheet(stylesheet);

expect(result).toBe(stylesheet);
});

it('should log a warning and report if a component tries to use a stylesheet that is not registered', () => {
const stylesheet = () => '';
function tmpl() {
return [];
}
tmpl.stylesheetToken = 'x-component_component';
tmpl.stylesheets = [stylesheet];
registerTemplate(tmpl);
class CustomElement extends LightningElement {}
registerComponent(CustomElement, { tmpl });

const elm = createElement('x-component', { is: CustomElement });

expect(() => {
document.body.appendChild(elm);
}).toLogWarningDev(
/\[LWC warn]: TypeError: Unexpected LWC stylesheet content found for component <x-component>./
);

expect(dispatcher).toHaveBeenCalled();
expect(dispatcher.calls.argsFor(0)).toEqual([
'UnexpectedStylesheetContent',
{
tagName: 'x-component',
},
]);
});

it('should not log a warning or report if a component tries to use a stylesheet that is registered', () => {
const stylesheet = () => '';
function tmpl() {
return [];
}
tmpl.stylesheetToken = 'x-component_component';
tmpl.stylesheets = [stylesheet];
registerStylesheet(stylesheet);
registerTemplate(tmpl);

class CustomElement extends LightningElement {}
registerComponent(CustomElement, { tmpl });

const elm = createElement('x-component', { is: CustomElement });

expect(() => {
document.body.appendChild(elm);
}).not.toLogError();
expect(dispatcher).not.toHaveBeenCalled();
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
import { createElement, __unstable__ReportingControl as reportingControl } from 'lwc';
import Test from 'x/test';

describe('issue-3156', () => {
let dispatcher;

beforeEach(() => {
dispatcher = jasmine.createSpy();
reportingControl.attachDispatcher(dispatcher);
});

afterEach(() => {
reportingControl.detachDispatcher();
});

it('logs a warning and reports when the engine attempts to evaluate an invalid function as a stylesheet', () => {
const element = createElement('x-test', { is: Test });
expect(() => {
document.body.appendChild(element);
}).toLogWarningDev(
/\[LWC warn]: TypeError: Unexpected LWC stylesheet content found for component <x-test>./
);

expect(dispatcher).toHaveBeenCalled();
expect(dispatcher.calls.argsFor(0)).toEqual([
'UnexpectedStylesheetContent',
{
tagName: 'x-test',
},
]);
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
function lib() {
return '';
}

export default [lib];
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
@import 'x/library';

.sample {
font-size: 5em;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
<template>
<div class="sample">Sample</div>
</template>
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
import { LightningElement } from 'lwc';

export default class Test extends LightningElement {}
Original file line number Diff line number Diff line change
Expand Up @@ -229,17 +229,32 @@ describe('programmatic stylesheets', () => {
expect(elm.shadowRoot.querySelector('h1')).toBeTruthy(); // still renders the template correctly
});

// TODO [#3122]: Disallow treating arbitrary functions as stylesheet functions
it('no error thrown if stylesheets is an array of arbitrary functions', () => {
// Disallow treating arbitrary functions as stylesheet functions
it('logs a warning if stylesheets is an array of arbitrary functions', () => {
let elm;
expect(() => {
elm = createElement('x-invalid3', {
is: Invalid3,
});
}).not.toLogErrorDev();

// Error is logged when the element is added to the body and the stylesheets are evaluated,
// not during createElement.
document.body.appendChild(elm);
}).toLogWarningDev(
/\[LWC warn]: TypeError: Unexpected LWC stylesheet content found for component <x-invalid3>./
);
});

// TODO [#3447]: This test verifies existing behavior is maintained and should be removed
// once the warnings are converted into errors.
it('applies the styles if the arbitrary function returns a valid string', () => {
const elm = createElement('x-invalid3', {
is: Invalid3,
});
document.body.appendChild(elm);
expect(elm.shadowRoot.querySelector('h1')).toBeTruthy(); // still renders the template correctly

const style = getComputedStyle(elm.shadowRoot.querySelector('h1'));
expect(style.color).toEqual('rgb(255, 0, 0)');
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { LightningElement } from 'lwc';
export default class extends LightningElement {
static stylesheets = [
() => {
return null;
return 'h1 { color: red; }';
},
];
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import {
createElement,
LightningElement,
registerTemplate,
registerStylesheet,
registerComponent,
__unstable__ReportingControl as reportingControl,
} from 'lwc';
Expand Down Expand Up @@ -87,16 +88,16 @@ if (!process.env.COMPAT) {
});

it('stylesheet', () => {
function stylesheet() {
return '';
/*LWC compiler v123.456.789*/
}
function tmpl() {
return [];
}
tmpl.stylesheetToken = 'x-component_component';
tmpl.stylesheets = [
function stylesheet() {
return '';
/*LWC compiler v123.456.789*/
},
];
tmpl.stylesheets = [stylesheet];
registerStylesheet(stylesheet);
registerTemplate(tmpl);
class CustomElement extends LightningElement {}
registerComponent(CustomElement, { tmpl });
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
import { registerStylesheet } from 'lwc';
function stylesheet(token, useActualHostSelector, useNativeDirPseudoclass) {
var shadowSelector = token ? ("[" + token + "]") : "";
var hostSelector = token ? ("[" + token + "-host]") : "";
var suffixToken = token ? ("-" + token) : "";
return "[aria-labelledby]" + shadowSelector + " {}[aria-labelledby=\"bar\"]" + shadowSelector + " {}";
/*LWC compiler vX.X.X*/
}
registerStylesheet(stylesheet);
export default [stylesheet];
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
import { registerStylesheet } from 'lwc';
function stylesheet(token, useActualHostSelector, useNativeDirPseudoclass) {
var shadowSelector = token ? ("[" + token + "]") : "";
var hostSelector = token ? ("[" + token + "-host]") : "";
var suffixToken = token ? ("-" + token) : "";
return "@charset \"utf-8\";@namespace url(http://www.w3.org/1999/xhtml);@keyframes slidein" + suffixToken + " {from {margin-left: 100%;}to {margin-left: 0%;}}@media screen and (min-width: 900px) {article" + shadowSelector + " {padding: 1rem 3rem;}}@supports (display: grid) {div" + shadowSelector + " {display: grid;}}@document url('https://www.example.com/') {h1" + shadowSelector + " {color: green;}}@font-face {font-family: 'Open Sans';src: url('/fonts/OpenSans-Regular-webfont.woff2') format('woff2'),\n url('/fonts/OpenSans-Regular-webfont.woff') format('woff');}@viewport {width: device-width;}@counter-style thumbs {system: cyclic;symbols: '\\1F44D';suffix: ' ';}@font-feature-values Font One {@styleset {nice-style: 12;}}";
/*LWC compiler vX.X.X*/
}
registerStylesheet(stylesheet);
export default [stylesheet];
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
import { registerStylesheet } from 'lwc';
function stylesheet(token, useActualHostSelector, useNativeDirPseudoclass) {
var shadowSelector = token ? ("[" + token + "]") : "";
var hostSelector = token ? ("[" + token + "-host]") : "";
var suffixToken = token ? ("-" + token) : "";
return ".foo" + shadowSelector + " {content: \"\\\\\";}";
/*LWC compiler vX.X.X*/
}
registerStylesheet(stylesheet);
export default [stylesheet];
Loading