Skip to content

Commit

Permalink
refactor(style): refactor code in compiler/style/css-imports (#3433)
Browse files Browse the repository at this point in the history 
This refactors and documents some code in the module responsible
for parsing css imports (src/compiler/style/css-imports.ts) and adds
some tests to increase coverage.

This commit also makes some edits to the general 'style' spec
(`src/compiler/style/test/style.spec.ts`) which has been skipped for a
while. The tests in this suite were 'end-to-end' unit tests of the style
handling capabilities of the compiler, which we'd like to have again,
but we can't reconstitute them until we get unit testing of the compiler
stood back up again.

So for now this ports a few tests out of `style.spec.ts` and into the
relevant specfiles (like `css-imports.spec.ts`) and deletes a few tests
from `style.spec.ts` which are no longer accurate (like testing for
escaping certain characters).
  • Loading branch information
@alicewriteswrongs
alicewriteswrongs authored Jul 5, 2022
1 parent 6effbc9 commit 4ad33a7
Show file tree
Hide file tree
Showing 9 changed files with 322 additions and 242 deletions.
41 changes: 36 additions & 5 deletions src/compiler/optimize/autoprefixer.ts
View file
Original file line number Diff line number Diff line change
@@ -1,9 +1,22 @@
import type * as d from '../../declarations';
import { IS_NODE_ENV, requireFunc } from '../sys/environment';
import { Postcss } from 'postcss';

let cssProcessor: any;
type CssProcessor = ReturnType<Postcss>;
let cssProcessor: CssProcessor;

export const autoprefixCss = async (cssText: string, opts: any) => {
/**
* Autoprefix a CSS string, adding vendor prefixes to make sure that what
* is written in the CSS will render correctly in our range of supported browsers.
* This function uses PostCSS in compbination with the Autoprefix plugin to
* automatically add vendor prefixes based on a list of browsers which we want
* to support.
*
* @param cssText the text to be prefixed
* @param opts an optional param with options for Autoprefixer
* @returns a Promise wrapping some prefixed CSS as well as diagnostics
*/
export const autoprefixCss = async (cssText: string, opts: boolean | null | d.AutoprefixerOptions) => {
const output: d.OptimizeCssOutput = {
output: cssText,
diagnostics: [],
Expand All @@ -13,7 +26,7 @@ export const autoprefixCss = async (cssText: string, opts: any) => {
}

try {
const autoprefixerOpts = opts != null && typeof opts === 'object' ? opts : DEFAULT_AUTOPREFIX_LEGACY;
const autoprefixerOpts = opts != null && typeof opts === 'object' ? opts : DEFAULT_AUTOPREFIX_OPTIONS;

const processor = getProcessor(autoprefixerOpts);
const result = await processor.process(cssText, { map: null });
Expand Down Expand Up @@ -79,15 +92,33 @@ export const autoprefixCss = async (cssText: string, opts: any) => {
return output;
};

const getProcessor = (autoprefixerOpts: any) => {
/**
* Get the processor for PostCSS and the Autoprefixer plugin
*
* @param autoprefixerOpts Options for Autoprefixer
* @returns postCSS with the Autoprefixer plugin applied
*/
const getProcessor = (autoprefixerOpts: d.AutoprefixerOptions): CssProcessor => {
const { postcss, autoprefixer } = requireFunc('../sys/node/autoprefixer.js');
if (!cssProcessor) {
cssProcessor = postcss([autoprefixer(autoprefixerOpts)]);
}
return cssProcessor;
};

const DEFAULT_AUTOPREFIX_LEGACY = {
/**
* Default options for the Autoprefixer PostCSS plugin. See the documentation:
* https://github.com/postcss/autoprefixer#options for a complete list.
*
* This default option set will:
*
* - override the default browser list (`overrideBrowserslist`)
* - turn off the visual cascade (`cascade`)
* - disable auto-removing outdated prefixes (`remove`)
* - set `flexbox` to `"no-2009"`, which limits prefixing for flexbox to the
* final and IE 10 versions of the specification
*/
const DEFAULT_AUTOPREFIX_OPTIONS: d.AutoprefixerOptions = {
overrideBrowserslist: ['last 2 versions', 'iOS >= 9', 'Android >= 4.4', 'Explorer >= 11', 'ExplorerMobile >= 11'],
cascade: false,
remove: false,
Expand Down
9 changes: 8 additions & 1 deletion src/compiler/optimize/optimize-css.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,14 @@ import { autoprefixCss } from './autoprefixer';
import { hasError } from '@utils';
import { minifyCss } from './minify-css';

export const optimizeCss = async (inputOpts: OptimizeCssInput) => {
/**
* Optimize a CSS file, optionally running an autoprefixer and a minifier
* depending on the options set on the input options argument.
*
* @param inputOpts input CSS options
* @returns a promise wrapping the optimized output
*/
export const optimizeCss = async (inputOpts: OptimizeCssInput): Promise<OptimizeCssOutput> => {
let result: OptimizeCssOutput = {
output: inputOpts.input,
diagnostics: [],
Expand Down
242 changes: 130 additions & 112 deletions src/compiler/style/css-imports.ts
View file
Original file line number Diff line number Diff line change
@@ -1,11 +1,24 @@
import type * as d from '../../declarations';
import { basename, dirname, isAbsolute, join, relative } from 'path';
import { basename, dirname, isAbsolute, join } from 'path';
import { buildError, normalizePath } from '@utils';
import { getModuleId } from '../sys/resolve/resolve-utils';
import { parseStyleDocs } from '../docs/style-docs';
import { resolveModuleIdAsync } from '../sys/resolve/resolve-module-async';
import { stripCssComments } from './style-utils';

/**
* Parse CSS imports into an object which contains a manifest of imports and a
* stylesheet with all imports resolved and concatenated.
*
* @param config the current config
* @param compilerCtx the compiler context (we need filesystem access)
* @param buildCtx the build context, we'll need access to diagnostics
* @param srcFilePath the source filepath
* @param resolvedFilePath the resolved filepath
* @param styleText style text we start with
* @param styleDocs optional array of style document objects
* @returns an object with concatenated styleText and imports
*/
export const parseCssImports = async (
config: d.Config,
compilerCtx: d.CompilerCtx,
Expand All @@ -14,113 +27,111 @@ export const parseCssImports = async (
resolvedFilePath: string,
styleText: string,
styleDocs?: d.StyleDoc[]
) => {
): Promise<ParseCSSReturn> => {
const isCssEntry = resolvedFilePath.toLowerCase().endsWith('.css');
const allCssImports: string[] = [];
const concatStyleText = await updateCssImports(
config,
compilerCtx,
buildCtx,
isCssEntry,
srcFilePath,
resolvedFilePath,
styleText,
allCssImports,
new Set(),
styleDocs
);

// a Set of previously-resolved file paths that we add to as we traverse the
// import tree (to avoid a possible circular dependency and infinite loop)
const resolvedFilePaths = new Set();

const concatStyleText = await resolveAndFlattenImports(srcFilePath, resolvedFilePath, styleText);

return {
imports: allCssImports,
styleText: concatStyleText,
};
};

const updateCssImports = async (
config: d.Config,
compilerCtx: d.CompilerCtx,
buildCtx: d.BuildCtx,
isCssEntry: boolean,
srcFilePath: string,
resolvedFilePath: string,
styleText: string,
allCssImports: string[],
noLoop: Set<string>,
styleDocs?: d.StyleDoc[]
) => {
if (noLoop.has(resolvedFilePath)) {
return styleText;
}
noLoop.add(resolvedFilePath);

if (styleDocs != null) {
parseStyleDocs(styleDocs, styleText);
}
/**
* Resolve and flatten all imports for a given CSS file, recursively crawling
* the tree of imports to resolve them all and produce a concatenated
* stylesheet. We declare this function here, within `parseCssImports`, in order
* to get access to `compilerCtx`, `buildCtx`, and more without having to pass
* a whole bunch of arguments.
*
* @param srcFilePath the source filepath
* @param resolvedFilePath the resolved filepath
* @param styleText style text we start with*
* @returns concatenated styles assembled from the various imported stylesheets
*/
async function resolveAndFlattenImports(
srcFilePath: string,
resolvedFilePath: string,
styleText: string
): Promise<string> {
// if we've seen this path before we early return
if (resolvedFilePaths.has(resolvedFilePath)) {
return styleText;
}
resolvedFilePaths.add(resolvedFilePath);

const cssImports = await getCssImports(config, compilerCtx, buildCtx, resolvedFilePath, styleText);
if (cssImports.length === 0) {
return styleText;
}
if (styleDocs != null) {
parseStyleDocs(styleDocs, styleText);
}

for (const cssImport of cssImports) {
if (!allCssImports.includes(cssImport.filePath)) {
allCssImports.push(cssImport.filePath);
const cssImports = await getCssImports(config, compilerCtx, buildCtx, resolvedFilePath, styleText);
if (cssImports.length === 0) {
return styleText;
}
}

await Promise.all(
cssImports.map(async (cssImportData) => {
await concatCssImport(
config,
compilerCtx,
buildCtx,
isCssEntry,
srcFilePath,
cssImportData,
allCssImports,
noLoop,
styleDocs
);
})
);

return replaceImportDeclarations(styleText, cssImports, isCssEntry);
};
// add any newly-found imports to the 'global' list
for (const cssImport of cssImports) {
if (!allCssImports.includes(cssImport.filePath)) {
allCssImports.push(cssImport.filePath);
}
}

const concatCssImport = async (
config: d.Config,
compilerCtx: d.CompilerCtx,
buildCtx: d.BuildCtx,
isCssEntry: boolean,
srcFilePath: string,
cssImportData: d.CssImportData,
allCssImports: string[],
noLoop: Set<string>,
styleDocs?: d.StyleDoc[]
) => {
cssImportData.styleText = await loadStyleText(compilerCtx, cssImportData);

if (typeof cssImportData.styleText === 'string') {
cssImportData.styleText = await updateCssImports(
config,
compilerCtx,
buildCtx,
isCssEntry,
cssImportData.filePath,
cssImportData.filePath,
cssImportData.styleText,
allCssImports,
noLoop,
styleDocs
// Recur down the tree of CSS imports, resolving all the imports in
// the children of the current file (and, by extension, in their children
// and so on)
await Promise.all(
cssImports.map(async (cssImportData) => {
cssImportData.styleText = await loadStyleText(compilerCtx, cssImportData);

if (typeof cssImportData.styleText === 'string') {
cssImportData.styleText = await resolveAndFlattenImports(
cssImportData.filePath,
cssImportData.filePath,
cssImportData.styleText
);
} else {
// we had some error loading the file from disk, so write a diagnostic
const err = buildError(buildCtx.diagnostics);
err.messageText = `Unable to read css import: ${cssImportData.srcImport}`;
err.absFilePath = srcFilePath;
}
})
);
} else {
const err = buildError(buildCtx.diagnostics);
err.messageText = `Unable to read css import: ${cssImportData.srcImport}`;
err.absFilePath = srcFilePath;

// replace import statements with the actual CSS code in children modules
return replaceImportDeclarations(styleText, cssImports, isCssEntry);
}
};

const loadStyleText = async (compilerCtx: d.CompilerCtx, cssImportData: d.CssImportData) => {
let styleText: string = null;
/**
* Interface describing the return value of `parseCSSImports`
*/
interface ParseCSSReturn {
/**
* An array of filepaths to the imported CSS files
*/
imports: string[];
/**
* The actual CSS text itself
*/
styleText: string;
}

/**
* Load the style text for a CSS file from disk, based on the filepaths set in
* our import data.
*
* @param compilerCtx the compiler context
* @param cssImportData the import data for the file we want to read
* @returns the contents of the file, if it can be read without error
*/
const loadStyleText = async (compilerCtx: d.CompilerCtx, cssImportData: d.CssImportData): Promise<string | null> => {
let styleText: string | null = null;

try {
styleText = await compilerCtx.fs.readFile(cssImportData.filePath);
Expand All @@ -135,6 +146,16 @@ const loadStyleText = async (compilerCtx: d.CompilerCtx, cssImportData: d.CssImp
return styleText;
};

/**
* Get a manifest of all the CSS imports in a given CSS file
*
* @param config the current config
* @param compilerCtx the compiler context (we need the filesystem)
* @param buildCtx the build context, in case we need to set a diagnostic
* @param filePath the filepath we're working with
* @param styleText the CSS for which we want to retrieve import data
* @returns a Promise wrapping a list of CSS import data objects
*/
export const getCssImports = async (
config: d.Config,
compilerCtx: d.CompilerCtx,
Expand All @@ -151,14 +172,15 @@ export const getCssImports = async (
styleText = stripCssComments(styleText);

const dir = dirname(filePath);
const importeeExt = filePath.split('.').pop().toLowerCase();
const importeeExt = (filePath.split('.').pop() ?? '').toLowerCase();

let r: RegExpExecArray;
let r: RegExpExecArray | null;
const IMPORT_RE = /(@import)\s+(url\()?\s?(.*?)\s?\)?([^;]*);?/gi;
while ((r = IMPORT_RE.exec(styleText))) {
const cssImportData: d.CssImportData = {
srcImport: r[0],
url: r[4].replace(/[\"\'\)]/g, ''),
filePath: '',
};

if (!isLocalCssImport(cssImportData.srcImport)) {
Expand Down Expand Up @@ -189,7 +211,10 @@ export const getCssImports = async (
}
}

if (typeof cssImportData.filePath === 'string') {
// we set `filePath` to `""` when the object is created above, so if it
// hasn't been changed in the intervening conditionals then we didn't resolve
// a filepath for it.
if (cssImportData.filePath !== '') {
imports.push(cssImportData);
}
}
Expand Down Expand Up @@ -244,23 +269,16 @@ export const isLocalCssImport = (srcImport: string) => {
return true;
};

export const replaceNodeModuleUrl = (
baseCssFilePath: string,
moduleId: string,
nodeModulePath: string,
url: string
) => {
nodeModulePath = normalizePath(dirname(nodeModulePath));
url = normalizePath(url);

const absPathToNodeModuleCss = normalizePath(url.replace(`~${moduleId}`, nodeModulePath));

const baseCssDir = normalizePath(dirname(baseCssFilePath));

const relToRoot = normalizePath(relative(baseCssDir, absPathToNodeModuleCss));
return relToRoot;
};

/**
* Replace import declarations (like '@import "foobar";') with the actual CSS
* written in the imported module, allowing us to produce a single file from a
* tree of stylesheets.
*
* @param styleText the text within which we want to replace @import statements
* @param cssImports information about imported modules
* @param isCssEntry whether we're dealing with a CSS file
* @returns an updated string with the requisite substitutions
*/
export const replaceImportDeclarations = (styleText: string, cssImports: d.CssImportData[], isCssEntry: boolean) => {
for (const cssImport of cssImports) {
if (isCssEntry) {
Expand Down
Loading

0 comments on commit 4ad33a7

Please sign in to comment.