Skip to content

Commit

Permalink
fix: improve performance of styling extractor
Browse files Browse the repository at this point in the history
  • Loading branch information
@guillaumerochelle
guillaumerochelle committed Dec 12, 2024
1 parent 2c094a7 commit 76dbe22
Show file tree
Hide file tree
Showing 14 changed files with 399 additions and 102 deletions.
2 changes: 2 additions & 0 deletions package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -255,8 +255,10 @@
"replace-in-files-cli": "^2.2.0",
"rimraf": "^5.0.1",
"sass": "~1.79.0",
"sass-embedded": "~1.79.0",
"sass-loader": "^14.0.0",
"semver": "^7.5.2",
"source-map-js": "^1.2.1",
"stylelint": "^16.0.2",
"stylelint-scss": "^6.0.0",
"ts-jest": "~29.2.0",
Expand Down
62 changes: 40 additions & 22 deletions packages/@o3r/styling/builders/style-extractor/helpers/css-variable.extractor.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,34 +5,27 @@ import * as fs from 'node:fs';
import { pathToFileURL } from 'node:url';
import * as path from 'node:path';
import {
compileString,
SassBoolean,
AsyncCompiler,
CalculationInterpolation, CalculationOperation, CalculationValue, initAsyncCompiler,
SassCalculation,
SassColor,
SassList,
SassMap,
SassNumber,
SassString,
StringOptions,
Value
} from 'sass';
} from 'sass-embedded';
import type { StyleExtractorBuilderSchema } from '../schema';

/**
* SassCalculation interface
*/
interface SassCalculation extends Value {
name: 'calc';
$arguments: string[];
}

/**
* CSS Variable extractor
*/
export class CssVariableExtractor {
static asyncCompiler: Promise<AsyncCompiler> = initAsyncCompiler();
private readonly cache: Record<string, URL> = {};

constructor(public defaultSassOptions?: StringOptions<'sync'>, private readonly builderOptions?: Pick<StyleExtractorBuilderSchema, 'ignoreInvalidValue'>) {

}

/**
Expand Down Expand Up @@ -80,7 +73,9 @@ export class CssVariableExtractor {
* @param color Sass Color
*/
private static getColorString(color: SassColor) {
return color.alpha ? `rgba(${color.red}, ${color.green}, ${color.blue}, ${color.alpha})` : `rgb(${color.red}, ${color.green}, ${color.blue}})`;
return color.alpha ?
`rgba(${color.channel('red')}, ${color.channel('green')}, ${color.channel('blue')}, ${color.alpha})` :
`rgb(${color.channel('red')}, ${color.channel('green')}, ${color.channel('blue')}})`;
}

/**
Expand Down Expand Up @@ -115,16 +110,35 @@ export class CssVariableExtractor {
return contextTags;
}

private static getCalcString(item: CalculationValue): string {
if (item) {
if (item instanceof SassNumber) {
const value = item.value;
const unit = item.numeratorUnits.get(0) ?? '';
return value + unit;
} else if (item instanceof SassString) {
return item.text;
} else if (item instanceof CalculationOperation) {
return `${CssVariableExtractor.getCalcString(item.left)} ${item.operator} ${CssVariableExtractor.getCalcString(item.right)}`;
} else if (item instanceof CalculationInterpolation) {
return item.value;
} else {
return `calc(${item.arguments.toArray().map((arg) => CssVariableExtractor.getCalcString(arg)).join(' ')})`;
}
}
return '';
}

/**
* Extract metadata from Sass Content
* @param sassFilePath SCSS file URL
* @param sassFileContent SCSS file content
* @param additionalSassOptions
*/
public extractFileContent(sassFilePath: string, sassFileContent: string, additionalSassOptions?: StringOptions<'sync'>) {
public async extractFileContent(sassFilePath: string, sassFileContent: string, additionalSassOptions?: StringOptions<'async'>) {
const cssVariables: CssVariable[] = [];

const options: StringOptions<'sync'> = {
const options: StringOptions<'async'> = {
...this.defaultSassOptions,
...additionalSassOptions,
loadPaths: [path.dirname(sassFilePath)],
Expand Down Expand Up @@ -219,7 +233,7 @@ export class CssVariableExtractor {
}

let parsedValue: string | undefined;
if (varValue instanceof SassString || varValue instanceof SassNumber || varValue instanceof SassBoolean) {
if (varValue instanceof SassString || varValue instanceof SassNumber) {
parsedValue = varValue.toString();
} else if (varValue instanceof SassColor) {
parsedValue = CssVariableExtractor.getColorString(varValue);
Expand All @@ -228,12 +242,12 @@ export class CssVariableExtractor {
const parsedValueItems: string[] = [];
for (let i = 0; i < varValue.asList.size; i++) {
const item = varValue.get(i);
if (item instanceof SassString || item instanceof SassNumber || item instanceof SassBoolean) {
if (item instanceof SassString || item instanceof SassNumber) {
parsedValueItems.push(item.toString());
} else if (item instanceof SassColor) {
parsedValueItems.push(CssVariableExtractor.getColorString(item));
} else if (CssVariableExtractor.isSassCalculation(item)) {
parsedValueItems.push(`calc(${item.$arguments[0]})`);
parsedValueItems.push(`calc(${item.arguments.toArray().map((arg) => CssVariableExtractor.getCalcString(arg)).join(' ')})`);
} else {
invalidIndexes.push(i);
}
Expand All @@ -248,7 +262,7 @@ export class CssVariableExtractor {
}
}
} else if (CssVariableExtractor.isSassCalculation(varValue)) {
parsedValue = `calc(${varValue.$arguments[0]})`;
parsedValue = `calc(${varValue.arguments.toArray().map((arg) => CssVariableExtractor.getCalcString(arg)).join(' ')})`;
} else if (!varValue.realNull) {
if (!details) {
console.warn(`The value "null" of ${varName.text} is available only for details override`);
Expand Down Expand Up @@ -293,17 +307,21 @@ export class CssVariableExtractor {
}
};

compileString(sassFileContent, options);
await (await CssVariableExtractor.asyncCompiler).compileStringAsync(sassFileContent, options);
return cssVariables;
}

public async disposeAsyncCompiler() {
await (await CssVariableExtractor.asyncCompiler).dispose();
}

/**
* Extract metadata from Sass file
* @param sassFilePath SCSS file to parse
*/
public extractFile(sassFilePath: string): CssVariable[] {
public async extractFile(sassFilePath: string): Promise<CssVariable[]> {
const sassFileContent = fs.readFileSync(sassFilePath, {encoding: 'utf8'});
return this.extractFileContent(sassFilePath, sassFileContent);
return await this.extractFileContent(sassFilePath, sassFileContent);
}

/**
Expand Down
28 changes: 12 additions & 16 deletions packages/@o3r/styling/builders/style-extractor/index.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,10 +57,10 @@ export default createBuilder(createBuilderWithMetricsIfInstalled<StyleExtractorB
/** CSS Metadata file to write */
let cssMetadata = (
// extract metadata for each file
await Promise.all(files.map((file, idx) => {
await Promise.all(files.map(async (file, idx) => {
try {
context.reportProgress(idx, STEP_NUMBER, `Extracting ${file}`);
const variables = cssVariableExtractor.extractFile(file);
const variables = await cssVariableExtractor.extractFile(file);
const themeFileSuffix = '.style.theme.scss';
if (file.endsWith(themeFileSuffix)) {
const componentPath = path.join(path.dirname(file), `${path.basename(file, themeFileSuffix)}.component.ts`);
Expand All @@ -83,19 +83,13 @@ export default createBuilder(createBuilderWithMetricsIfInstalled<StyleExtractorB
}
}))
).reduce<CssMetadata>((acc, cssVarList) => {
// Check duplicate CSS variable
cssVarList
.filter((cssVar) => !!acc.variables[cssVar.name])
.filter((cssVar) => !initialPreviousMetadata.variables[cssVar.name] && acc.variables[cssVar.name].defaultValue !== cssVar.defaultValue)
.forEach((cssVar) =>
context.logger[options.ignoreDuplicateWarning ? 'debug' : 'warn'](`Duplicate "${cssVar.name}" (${acc.variables[cssVar.name].defaultValue} will be replaced by ${cssVar.defaultValue})`)
);
for (const cssVar of cssVarList) {
if (!!acc.variables[cssVar.name] && !initialPreviousMetadata.variables[cssVar.name] && acc.variables[cssVar.name].defaultValue !== cssVar.defaultValue) {
context.logger[options.ignoreDuplicateWarning ? 'debug' : 'warn'](`Duplicate "${cssVar.name}" (${acc.variables[cssVar.name].defaultValue} will be replaced by ${cssVar.defaultValue})`);
}
acc.variables[cssVar.name] = cssVar;
}

// merge all variables form all the files
cssVarList
.forEach((item) => {
acc.variables[item.name] = item;
});
return acc;
}, previousMetadata);

Expand Down Expand Up @@ -175,8 +169,9 @@ export default createBuilder(createBuilderWithMetricsIfInstalled<StyleExtractorB
};

if (!options.watch) {
return execute(getAllFiles());

const result = execute(getAllFiles());
void cssVariableExtractor.disposeAsyncCompiler();
return result;
} else {
/** Cache */
const cacheMetadata: CssMetadata = {
Expand Down Expand Up @@ -221,6 +216,7 @@ export default createBuilder(createBuilderWithMetricsIfInstalled<StyleExtractorB
});

context.addTeardown(async () => {
await cssVariableExtractor.disposeAsyncCompiler();
await watcher.close();
await metadataWatcher.close();
});
Expand Down
6 changes: 5 additions & 1 deletion packages/@o3r/styling/package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,9 @@
"@yarnpkg/plugin-pack": "^4.0.0",
"rxjs": "^7.8.1",
"sass": "~1.79.0",
"semver": "^7.5.2"
"sass-embedded": "~1.79.0",
"semver": "^7.5.2",
"source-map-js": "^1.2.1"
},
"peerDependenciesMeta": {
"@angular-devkit/architect": {
Expand Down Expand Up @@ -180,7 +182,9 @@
"pid-from-port": "^1.1.3",
"rxjs": "^7.8.1",
"sass": "~1.79.0",
"sass-embedded": "~1.79.0",
"semver": "^7.5.2",
"source-map-js": "^1.2.1",
"stylelint": "^16.0.2",
"stylelint-scss": "^6.0.0",
"ts-jest": "~29.2.0",
Expand Down
47 changes: 21 additions & 26 deletions tools/github-actions/audit/packaged-action/index.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26205,7 +26205,7 @@ const colors = ['', 'green', 'yellow', 'orange', 'red'];
*/
function computeYarn4Report(response, severityThreshold) {
core.info('Computing Report for Yarn 4');
const reports = response.split('\n').filter((a) => !!a);
const reports = response.split('\n').filter(a => !!a);
const severityThresholdIndex = severities.indexOf(severityThreshold);
return reports.reduce((currentReport, currentVulnerability) => {
const vulnerabilityReport = JSON.parse(currentVulnerability);
Expand All @@ -26225,9 +26225,8 @@ function computeYarn4Report(response, severityThreshold) {
overview: `This issue affects versions ${vulnerabilityReport.children['Vulnerable Versions']}. ${vulnerabilityReport.children.Issue}`
});
}
currentReport.highestSeverityFound = severities.indexOf(currentReport.highestSeverityFound || 'info') <= severities.indexOf(vulnerabilitySeverity)
? vulnerabilitySeverity
: currentReport.highestSeverityFound;
currentReport.highestSeverityFound = severities.indexOf(currentReport.highestSeverityFound || 'info') <= severities.indexOf(vulnerabilitySeverity) ?
vulnerabilitySeverity : currentReport.highestSeverityFound;
currentReport.nbVulnerabilities += 1;
return currentReport;
}, { nbVulnerabilities: 0, errors: [], warnings: [] });
Expand Down Expand Up @@ -26276,13 +26275,13 @@ async function run() {
core.warning(err);
core.setOutput('reportJSON', report);
const reportData = version >= 4 ? computeYarn4Report(report, severityConfig) : computeYarn3Report(report, severityConfig);
if (reportData.highestSeverityFound) {
core.info(`Highest severity found: ${reportData.highestSeverityFound}`);
}
else {
if (!reportData.highestSeverityFound) {
core.info('No vulnerability detected.');
return;
}
else {
core.info(`Highest severity found: ${reportData.highestSeverityFound}`);
}
const isFailed = reportData.errors.length > 0;
const getBadge = (sev) => `![${sev}](https://img.shields.io/static/v1?label=&logo=npm&message=${sev}&color=${colors[severities.indexOf(sev)]})`;
const formatVulnerability = (vulnerability) => `
Expand All @@ -26295,40 +26294,36 @@ ${vulnerability.overview.replaceAll('### ', '#### ')}
</details>

`;
const isVulnerabilityWithKnownSeverity = (advisory) => severities.includes(advisory.severity);
const isVulnerabilityWithKnownSeverity = (advisory) => severities.indexOf(advisory.severity) >= 0;
const sortVulnerabilityBySeverity = (advisory1, advisory2) => severities.indexOf(advisory2.severity) - severities.indexOf(advisory1.severity);
const body = `# Audit report ${isFailed ? ':x:' : ':white_check_mark:'}

${reportData.nbVulnerabilities} vulnerabilities found.

${reportData.errors.length > 0
? `## Vulnerabilities to be fixed
${reportData.errors.length ? `## Vulnerabilities to be fixed

${reportData.errors
.filter(isVulnerabilityWithKnownSeverity)
.sort(sortVulnerabilityBySeverity)
.map(formatVulnerability)
.join(os.EOL)}
`
: ''}
${reportData.warnings.length > 0
? `___
.filter(isVulnerabilityWithKnownSeverity)
.sort(sortVulnerabilityBySeverity)
.map(formatVulnerability)
.join(os.EOL)}
` : ''}
${reportData.warnings.length ? `___

<details>
<summary>
Vulnerabilities below the threshold: ${severityConfig}
</summary>

${reportData.warnings
.filter(isVulnerabilityWithKnownSeverity)
.sort(sortVulnerabilityBySeverity)
.map(formatVulnerability)
.join(os.EOL)
.replaceAll('${', '&#36;{')}
.filter(isVulnerabilityWithKnownSeverity)
.sort(sortVulnerabilityBySeverity)
.map(formatVulnerability)
.join(os.EOL)
.replaceAll('${', '&#36;{')}

</details>
`
: ''}
` : ''}
`;
core.setOutput('reportMarkdown', body);
if (isFailed) {
Expand Down
2 changes: 1 addition & 1 deletion tools/github-actions/audit/packaged-action/index.js.map
View file

Large diffs are not rendered by default.

Loading

0 comments on commit 76dbe22

Please sign in to comment.