Skip to content
Open
Show file tree
Hide file tree
Changes from 4 commits
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
1 change: 1 addition & 0 deletions src/compiler/build/build-hmr.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ export const generateHmr = (config: d.Config, compilerCtx: d.CompilerCtx, buildC
styleId: getScopeId(s.styleTag, s.styleMode),
styleTag: s.styleTag,
styleText: s.styleText,
styleSourceMap: s.styleSourceMap,
} as d.HmrStyleUpdate;
}),
(s) => s.styleId,
Expand Down
2 changes: 1 addition & 1 deletion src/compiler/bundle/app-data-plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ export const appDataPlugin = (
});
buildCtx.diagnostics.push(...loadTypeScriptDiagnostics(results.diagnostics));

if (config.sourceMap) {
if (config.sourceMapJs) {
// generate the sourcemap for global script
const codeMs = new MagicString(code);
const codeMap = codeMs.generateMap({
Expand Down
2 changes: 1 addition & 1 deletion src/compiler/bundle/bundle-output.ts
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,7 @@ export const getRollupOptions = (
resolveIdWithTypeScript(config, compilerCtx),
rollupCommonjsPlugin({
include: /node_modules/,
sourceMap: config.sourceMap,
sourceMap: config.sourceMapJs,
transformMixedEsModules: false,
...config.commonjs,
}),
Expand Down
75 changes: 46 additions & 29 deletions src/compiler/bundle/ext-transforms-plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,33 @@ import { parseImportPath } from '../transformers/stencil-import-path';
* }
* ```
*
* Whenever one of the files change, we can propagate a correct concatenated
* version of all styles to the browser by setting `buildCtx.stylesUpdated`.
* Whenever one of the files changes, we can propagate the complete ordered
* style list and its matching source maps to the browser via `buildCtx.stylesUpdated`.
*/
type ComponentStyleMap = Map<string, string>;
type ComponentStyle = {
styleText: string;
styleSourceMap?: string;
};
type ComponentStyleMap = Map<string, ComponentStyle>;
const allCmpStyles = new Map<string, ComponentStyleMap>();

const getComponentStyleEntries = (
cmpStyles: ComponentStyleMap | undefined,
externalStyles: d.ExternalStyleCompiler[] | undefined,
fallbackStyle: ComponentStyle,
): Array<ComponentStyle | undefined> => {
if (!cmpStyles) {
return [fallbackStyle];
}
if (!externalStyles) {
return [...cmpStyles.values()];
}

return externalStyles.map((externalStyle) => {
return cmpStyles.get(externalStyle.originalComponentPath) || cmpStyles.get(externalStyle.absolutePath);
});
};

/**
* A Rollup plugin which bundles up some transformation of CSS imports as well
* as writing some files to disk for the `DIST_COLLECTION` output target.
Expand Down Expand Up @@ -128,7 +149,8 @@ export const extTransformsPlugin = (
addTagTransformers: !!buildCtx.config.extras.additionalTagTransformers,
encapsulation: data.encapsulation,
mode: data.mode,
sourceMap: config.sourceMap,
sourceMap: config.sourceMapCss,
map: pluginTransforms.map,
minify: config.minifyCss,
autoprefixer: config.autoprefixCss,
docs: config.buildDocs,
Expand All @@ -138,7 +160,10 @@ export const extTransformsPlugin = (
* persist component styles for transformed stylesheet
*/
if (cmpStyles) {
cmpStyles.set(filePath, cssTransformResults.styleText);
cmpStyles.set(filePath, {
styleText: cssTransformResults.styleText,
styleSourceMap: cssTransformResults.styleSourceMap,
});
}

// Set style docs
Expand Down Expand Up @@ -170,35 +195,27 @@ export const extTransformsPlugin = (
if (!hasUpdatedStyle && data.tag && data.mode) {
const externalStyles = cmp?.styles?.[0]?.externalStyles;

/**
* if component has external styles, use a list to keep the order to which
* styles are applied.
*/
const styleText = cmpStyles
? externalStyles
? /**
* attempt to find the original `filePath` key through `originalComponentPath`
* and `absolutePath` as path can differ based on how Stencil is installed
* e.g. through `npm link` or `npm install`
*/
externalStyles
.map((es) => cmpStyles.get(es.originalComponentPath) || cmpStyles.get(es.absolutePath))
.join('\n')
: /**
* if `externalStyles` is not defined, then created the style text in the
* order of which the styles were compiled.
*/
[...cmpStyles.values()].join('\n')
: /**
* if `cmpStyles` is not defined, then use the style text from the transform
* as it is not connected to a component.
*/
cssTransformResults.styleText;
const styleEntries = getComponentStyleEntries(cmpStyles, externalStyles, {
styleText: cssTransformResults.styleText,
styleSourceMap: cssTransformResults.styleSourceMap,
});
const separateStyles = config.sourceMapCss && styleEntries.length > 1;
const styleText = separateStyles
? styleEntries.map((entry) => entry?.styleText || '')
: styleEntries.map((entry) => entry?.styleText || '').join('\n');
const styleSourceMap = !config.sourceMapCss
? undefined
: styleEntries.length === 1
? styleEntries[0]?.styleSourceMap
: styleEntries.every((entry) => entry?.styleSourceMap != null)
? styleEntries.map((entry) => entry.styleSourceMap!)
: undefined;

buildCtx.stylesUpdated.push({
styleTag: data.tag,
styleMode: data.mode,
styleText,
styleSourceMap,
});
}

Expand Down
75 changes: 73 additions & 2 deletions src/compiler/bundle/test/ext-transforms-plugin.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ describe('extTransformsPlugin', () => {
writeFileSpy,
transformCssToEsmSpy,
cssText,
compilerComponentMeta,
};
}

Expand All @@ -81,10 +82,10 @@ describe('extTransformsPlugin', () => {

it('should return early if no data can be gleaned from the id', async () => {
const { plugin } = setup();
// @ts-ignore we're testing something which shouldn't normally happen,
// @ts-expect-error we're testing something which shouldn't normally happen,
// but might if an argument of the wrong type were passed as `id`
const parseSpy = jest.spyOn(importPathLib, 'parseImportPath').mockReturnValue({ data: null });
// @ts-ignore the Rollup plugins expect to be called in a Rollup context
// @ts-expect-error the Rollup plugins expect to be called in a Rollup context
expect(await plugin.transform('asdf', 'foo.css')).toBe(null);
parseSpy.mockRestore();
});
Expand All @@ -101,5 +102,75 @@ describe('extTransformsPlugin', () => {

expect(css).toBe(':host { text: pink; }');
});

it('should pass the CSS source-map setting and plugin map to the CSS transform', async () => {
const { plugin, config, transformCssToEsmSpy } = setup();
const map = {
version: 3,
file: '/some/stubbed/path/foo.css',
names: [] as string[],
mappings: 'AAAA',
sources: ['/some/stubbed/path/foo.scss'],
sourcesContent: [':host { text: pink; }'],
};

config.sourceMap = { css: true, js: false };
config.sourceMapCss = true;
config.plugins = [
{
transform: () => ({
code: ':host { text: pink; }',
map: JSON.stringify(map),
}),
},
];

// @ts-expect-error the Rollup plugins expect to be called in a Rollup context
await plugin.transform('asdf', '/some/stubbed/path/foo.css?tag=my-component');

expect(transformCssToEsmSpy).toHaveBeenCalledWith(
expect.objectContaining({
sourceMap: true,
map,
}),
);
});

it('should keep source-mapped external styles in styleUrl order', async () => {
const { plugin, config, buildCtx, compilerComponentMeta, transformCssToEsmSpy } = setup();
const firstStylePath = '/some/stubbed/path/first.css';
const secondStylePath = '/some/stubbed/path/second.css';
compilerComponentMeta.styles = [
{
modeName: 'default',
externalStyles: [
{ absolutePath: firstStylePath, originalComponentPath: firstStylePath },
{ absolutePath: secondStylePath, originalComponentPath: secondStylePath },
],
},
] as any;
config.sourceMap = { css: true, js: false };
config.sourceMapCss = true;
transformCssToEsmSpy.mockImplementation(async ({ file }) => ({
styleText: file === firstStylePath ? 'FIRST' : 'SECOND',
styleSourceMap: file === firstStylePath ? 'FIRST_MAP' : 'SECOND_MAP',
output: '',
map: null,
diagnostics: [],
imports: [],
defaultVarName: 'foo',
styleDocs: [],
}));

// Process the second style first to prove HMR updates do not use transform completion order.
// @ts-expect-error the Rollup plugins expect to be called in a Rollup context
await plugin.transform('asdf', `${secondStylePath}?tag=my-component`);
// @ts-expect-error the Rollup plugins expect to be called in a Rollup context
await plugin.transform('asdf', `${firstStylePath}?tag=my-component`);

const latestUpdate = buildCtx.stylesUpdated[buildCtx.stylesUpdated.length - 1];
expect(latestUpdate.styleText).toEqual(['FIRST', 'SECOND']);
expect(latestUpdate.styleSourceMap).toEqual(['FIRST_MAP', 'SECOND_MAP']);
});
});
});
39 changes: 33 additions & 6 deletions src/compiler/config/test/validate-config-sourcemap.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ describe('stencil config - sourceMap option', () => {
const { config: validatedConfig } = validateConfig(userConfig, {});
expect(validatedConfig.sourceMap).toBe(true);

mockTsConfigParser(validatedConfig.sourceMap);
mockTsConfigParser(validatedConfig.sourceMapJs);
const testConfig = getLoadConfigForTests({ config: userConfig });
const loadConfigResults = await loadConfig(testConfig);

Expand All @@ -73,7 +73,7 @@ describe('stencil config - sourceMap option', () => {
const { config: validatedConfig } = validateConfig(userConfig, {});
expect(validatedConfig.sourceMap).toBe(false);

mockTsConfigParser(validatedConfig.sourceMap);
mockTsConfigParser(validatedConfig.sourceMapJs);
const testConfig = getLoadConfigForTests({ config: userConfig });
const loadConfigResults = await loadConfig(testConfig);

Expand All @@ -87,7 +87,7 @@ describe('stencil config - sourceMap option', () => {
const { config: validatedConfig } = validateConfig(userConfig, {});
expect(validatedConfig.sourceMap).toBe(false);

mockTsConfigParser(validatedConfig.sourceMap);
mockTsConfigParser(validatedConfig.sourceMapJs);
const testConfig = getLoadConfigForTests({ config: userConfig });
const loadConfigResults = await loadConfig(testConfig);

Expand All @@ -101,7 +101,7 @@ describe('stencil config - sourceMap option', () => {
const { config: validatedConfig } = validateConfig(userConfig, {});
expect(validatedConfig.sourceMap).toBe(true);

mockTsConfigParser(validatedConfig.sourceMap);
mockTsConfigParser(validatedConfig.sourceMapJs);
const testConfig = getLoadConfigForTests({ config: userConfig });
const loadConfigResults = await loadConfig(testConfig);

Expand All @@ -115,7 +115,7 @@ describe('stencil config - sourceMap option', () => {
const { config: validatedConfig } = validateConfig(userConfig, {});
expect(validatedConfig.sourceMap).toBe(true);

mockTsConfigParser(validatedConfig.sourceMap);
mockTsConfigParser(validatedConfig.sourceMapJs);
const testConfig = getLoadConfigForTests({ config: userConfig });
const loadConfigResults = await loadConfig(testConfig);

Expand All @@ -129,12 +129,39 @@ describe('stencil config - sourceMap option', () => {
const { config: validatedConfig } = validateConfig(userConfig, {});
expect(validatedConfig.sourceMap).toBe(false);

mockTsConfigParser(validatedConfig.sourceMap);
mockTsConfigParser(validatedConfig.sourceMapJs);
const testConfig = getLoadConfigForTests({ config: userConfig });
const loadConfigResults = await loadConfig(testConfig);

const { sourceMap, inlineSources } = loadConfigResults.config.tsCompilerOptions;
expect(sourceMap).toBe(false);
expect(inlineSources).toBe(false);
});

it('enables only CSS source maps when configured with css', () => {
const userConfig: d.UnvalidatedConfig = { sourceMap: { css: true }, devMode: false };
const { config: validatedConfig } = validateConfig(userConfig, {});

expect(validatedConfig.sourceMap).toEqual({ css: true });
expect(validatedConfig.sourceMapCss).toBe(true);
expect(validatedConfig.sourceMapJs).toBe(false);
});

it('enables only JavaScript source maps when configured with js', () => {
const userConfig: d.UnvalidatedConfig = { sourceMap: { js: true }, devMode: false };
const { config: validatedConfig } = validateConfig(userConfig, {});

expect(validatedConfig.sourceMap).toEqual({ js: true });
expect(validatedConfig.sourceMapCss).toBe(false);
expect(validatedConfig.sourceMapJs).toBe(true);
});

it('keeps legacy boolean source maps enabled for JavaScript only', () => {
const userConfig: d.UnvalidatedConfig = { sourceMap: true, devMode: false };
const { config: validatedConfig } = validateConfig(userConfig, {});

expect(validatedConfig.sourceMap).toBe(true);
expect(validatedConfig.sourceMapCss).toBe(false);
expect(validatedConfig.sourceMapJs).toBe(true);
});
});
1 change: 1 addition & 0 deletions src/compiler/config/transpile-options.ts
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,7 @@ export const getTranspileConfig = (input: TranspileOptions): TranspileConfig =>
file: compileOpts.file,
style: compileOpts.style as any,
styleImportData: compileOpts.styleImportData as any,
sourceMapCss: compileOpts.sourceMap !== false,

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

do we need to check if sourceMap.css is set as well?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

This is the legacy TranspileOptions path, whose sourceMap option is boolean or inline; it does not use the validated build sourceMap.css object. sourceMapCss is therefore derived from compileOpts.sourceMap !== false here.

target: compileOpts.target as any,
};

Expand Down
21 changes: 19 additions & 2 deletions src/compiler/config/validate-config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,7 @@ export const validateConfig = (
}

const hashFileNames = config.hashFileNames ?? !devMode;
const sourceMap = validateSourceMap(config.sourceMap, devMode);

const validatedConfig: ValidatedConfig = {
devServer: {}, // assign `devServer` before spreading `config`, in the event 'devServer' is not a key on `config`
Expand All @@ -135,8 +136,7 @@ export const validateConfig = (
minifyJs: config.minifyJs ?? !devMode,
outputTargets: config.outputTargets ?? [],
rollupConfig: validateRollupConfig(config),
sourceMap:
config.sourceMap === true || (devMode && (config.sourceMap === 'dev' || typeof config.sourceMap === 'undefined')),
...sourceMap,
sys: config.sys ?? bootstrapConfig.sys ?? createNodeSys({ logger }),
testing: config.testing ?? {},
docs: validateDocs(config, logger),
Expand Down Expand Up @@ -299,3 +299,20 @@ export const validateConfig = (
diagnostics,
};
};

const validateSourceMap = (sourceMap: UnvalidatedConfig['sourceMap'], devMode: boolean) => {
if (sourceMap != null && typeof sourceMap === 'object') {
return {
sourceMap,
sourceMapCss: sourceMap.css === true,
sourceMapJs: sourceMap.js === true,
};
}

const enabled = sourceMap === true || (devMode && (sourceMap === 'dev' || sourceMap == null));
return {
sourceMap: enabled,
sourceMapCss: false,
sourceMapJs: enabled,
};
};
Loading