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

feat(rollup-plugin): warn stylesheet import for CSS extensibility #3270

Merged
merged 5 commits into from
Jan 10, 2023
Merged

feat(rollup-plugin): warn stylesheet import for CSS extensibility #3270

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
3977e91
feat(rollup-plugin): enforce stylesheet module import
rui-rayqiu Jan 6, 2023
6beed23
fix: add a test
rui-rayqiu Jan 6, 2023
6a27ea9
fix: fix a typo in test
rui-rayqiu Jan 6, 2023
30e6501
fix: address PR comment, remove observable change
rui-rayqiu Jan 6, 2023
2717caf
fix: minor fix on warning message and test
rui-rayqiu Jan 10, 2023
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
6 changes: 6 additions & 0 deletions packages/@lwc/rollup-plugin/src/__tests__/resolver/fixtures/missing-css/missing-css.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
import { LightningElement } from 'lwc';
import stylesheet from './stylesheet.css';

export default class MissingCss extends LightningElement {
static stylesheets = [stylesheet];
}
22 changes: 22 additions & 0 deletions packages/@lwc/rollup-plugin/src/__tests__/resolver/resolver.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -161,4 +161,26 @@ describe('resolver', () => {

expect(warnings).toHaveLength(0);
});

it('should emit a warning when import stylesheet file is missing', async () => {
const warnings: any = [];

const bundle = await rollup({
input: path.resolve(__dirname, 'fixtures/missing-css/missing-css.js'),
plugins: [lwc()],
onwarn(warning) {
warnings.push(warning);
},
});

const { output } = await bundle.generate({
format: 'esm',
});

expect(warnings).toHaveLength(1);
expect(warnings[0].message).toMatch(
/The imported CSS file .+\/stylesheet.css does not exist: Importing it as undefined./
);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we check that the code is still compiled successfully here?

expect(output[0].code).toContain('var stylesheet = undefined;');
});
nolanlawson marked this conversation as resolved.
Show resolved Hide resolved
});
45 changes: 37 additions & 8 deletions packages/@lwc/rollup-plugin/src/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,8 @@ const DEFAULT_MODULES = [

const IMPLICIT_DEFAULT_HTML_PATH = '@lwc/resources/empty_html.js';
const EMPTY_IMPLICIT_HTML_CONTENT = 'export default void 0';
const IMPLICIT_DEFAULT_CSS_PATH = '@lwc/resources/empty_css.css';
const EMPTY_IMPLICIT_CSS_CONTENT = '';

function isImplicitHTMLImport(importee: string, importer: string): boolean {
return (
Expand All @@ -57,6 +59,15 @@ function isImplicitHTMLImport(importee: string, importer: string): boolean {
);
}

function isImplicitCssImport(importee: string, importer: string): boolean {
return (
path.extname(importee) === '.css' &&
path.extname(importer) === '.html' &&
(path.basename(importee, '.css') === path.basename(importer, '.html') ||
path.basename(importee, '.scoped.css') === path.basename(importer, '.html'))
);
}

interface scopedOption {
filename: string;
scoped: boolean;
Expand Down Expand Up @@ -158,12 +169,19 @@ export default function lwc(pluginOptions: RollupLwcOptions = {}): Plugin {
const ext = path.extname(importee) || importerExt;

const normalizedPath = path.resolve(path.dirname(importer), importee);
const absPath = pluginUtils.addExtension(normalizedPath, ext);
let absPath = pluginUtils.addExtension(normalizedPath, ext);

const { filename } = parseQueryParamsForScopedOption(absPath);
absPath = filename; // remove query param

if (isImplicitHTMLImport(normalizedPath, importer) && !fs.existsSync(absPath)) {
if (isImplicitHTMLImport(absPath, importer) && !fs.existsSync(absPath)) {
return IMPLICIT_DEFAULT_HTML_PATH;
}

if (isImplicitCssImport(absPath, importer) && !fs.existsSync(absPath)) {
return IMPLICIT_DEFAULT_CSS_PATH;
}

return pluginUtils.addExtension(normalizedPath, ext);
} else if (importer) {
// Could be an import like `import component from 'x/component'`
Expand All @@ -185,18 +203,29 @@ export default function lwc(pluginOptions: RollupLwcOptions = {}): Plugin {
return EMPTY_IMPLICIT_HTML_CONTENT;
}

if (id === IMPLICIT_DEFAULT_CSS_PATH) {
return EMPTY_IMPLICIT_CSS_CONTENT;
}

// Have to parse the `?scoped=true` in `load`, because it's not guaranteed
// that `resolveId` will always be called (e.g. if another plugin resolves it first)
const { scoped, filename } = parseQueryParamsForScopedOption(id);
if (scoped) {
id = filename; // remove query param
}
const { filename } = parseQueryParamsForScopedOption(id);
id = filename; // remove query param

const isCSS = path.extname(id) === '.css';

if (isCSS) {
const exists = fs.existsSync(id);
const code = exists ? fs.readFileSync(id, 'utf8') : '';
return code;
if (exists) {
return fs.readFileSync(id, 'utf8');
} else {
this.warn(
`The imported CSS file ${id} does not exist: Importing it as undefined. ` +
`This behavior may be removed in a future version of LWC. Please avoid importing a ` +
`CSS file that does not exist.`
);
return EMPTY_IMPLICIT_CSS_CONTENT;
}
}
},

Expand Down