Skip to content

Commit

Permalink
Added partial caching to GS100 and GS110 rules
Browse files Browse the repository at this point in the history
refs 0f0b062

- this adds partial caching, similar to the commit above, so we don't
  perform unnecessary work in GScan theme checking
  • Loading branch information
daniellockyer committed Oct 7, 2024
1 parent 0f35572 commit f57ab07
Show file tree
Hide file tree
Showing 2 changed files with 39 additions and 10 deletions.
22 changes: 18 additions & 4 deletions lib/checks/100-custom-template-settings-usage.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ function getLogger({theme, rule, file = null}) {
}

function applyRule(rule, theme) {
const partialVerificationCache = new Map();
// The result variable is passed around to keep a state through the full lifecycle
const result = {};
try {
Expand All @@ -56,7 +57,7 @@ function applyRule(rule, theme) {
// Run the main function on each theme file (optional)
if (typeof rule.eachFile === 'function') {
_.each(theme.files , function (themeFile) {
rule.eachFile({file: themeFile, theme, log: getLogger({theme, rule}), result});
rule.eachFile({file: themeFile, theme, log: getLogger({theme, rule}), result, partialVerificationCache});
});
}

Expand All @@ -71,7 +72,7 @@ function applyRule(rule, theme) {
}
}

function parseWithAST({theme, log, file, rules, callback}){
function parseWithAST({theme, log, file, rules, callback, partialVerificationCache}){
const linter = new ASTLinter();

// This rule is needed to find partials
Expand All @@ -86,13 +87,26 @@ function parseWithAST({theme, log, file, rules, callback}){
return;
}

const fileName = themeFile.file;
// Check if the file is a partial
const isPartial = fileName.startsWith('partials/');
// Skip if already cached (for partials only)
if (isPartial && partialVerificationCache.has(fileName)) {
return;
}

const astResults = linter.verify({
parsed: themeFile.parsed,
rules,
source: themeFile.content,
moduleId: themeFile.file
});

// Cache the result for this partial
if (isPartial) {
partialVerificationCache.set(fileName, astResults);
}

if (astResults.length) {
log.failure({
message: astResults[0].message
Expand Down Expand Up @@ -122,13 +136,13 @@ const ruleImplementations = {
init: ({result}) => {
result.customThemeSettings = new Set();
},
eachFile: ({file, theme, log, result}) => {
eachFile: ({file, theme, log, result, partialVerificationCache}) => {
let templateTest = file.file.match(/(?<!partials\/.+?)\.hbs$/);

if (templateTest) {
parseWithAST({file, theme, rules: {
'mark-used-custom-theme-setting': require(`../ast-linter/rules/mark-used-custom-theme-settings`)
}, log, callback: (linter) => {
}, log, partialVerificationCache, callback: (linter) => {
linter.customThemeSettings.forEach((variable) => {
result.customThemeSettings.add(variable);
});
Expand Down
27 changes: 21 additions & 6 deletions lib/checks/110-page-builder-usage.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,8 @@ function getLogger({theme, rule, file = null}) {
}

function applyRule(rule, theme) {
const partialVerificationCache = new Map();

// The result variable is passed around to keep a state through the full lifecycle
const result = {};
try {
Expand All @@ -56,7 +58,7 @@ function applyRule(rule, theme) {
// Run the main function on each theme file (optional)
if (typeof rule.eachFile === 'function') {
_.each(theme.files, function (themeFile) {
rule.eachFile({file: themeFile, theme, log: getLogger({theme, rule}), result});
rule.eachFile({file: themeFile, theme, log: getLogger({theme, rule}), result, partialVerificationCache});
});
}

Expand All @@ -71,7 +73,7 @@ function applyRule(rule, theme) {
}
}

function parseWithAST({theme, log, file, rules, callback}) {
function parseWithAST({theme, log, file, rules, callback, partialVerificationCache}) {
const linter = new ASTLinter();

// This rule is needed to find partials
Expand All @@ -86,13 +88,26 @@ function parseWithAST({theme, log, file, rules, callback}) {
return;
}

const fileName = themeFile.file;
// Check if the file is a partial
const isPartial = fileName.startsWith('partials/');
// Skip if already cached (for partials only)
if (isPartial && partialVerificationCache.has(fileName)) {
return;
}

const astResults = linter.verify({
parsed: themeFile.parsed,
rules,
source: themeFile.content,
moduleId: themeFile.file
});

// Cache the result for this partial
if (isPartial) {
partialVerificationCache.set(fileName, astResults);
}

if (astResults.length) {
log.failure({
message: astResults[0].message,
Expand Down Expand Up @@ -121,13 +136,13 @@ const ruleImplementations = {
init: ({result}) => {
result.pageBuilderProperties = new Set();
},
eachFile: ({file, theme, log, result}) => {
eachFile: ({file, theme, log, result, partialVerificationCache}) => {
const templateTest = file.file.match(/(?<!partials\/.+?)\.hbs$/);

if (templateTest) {
parseWithAST({file, theme, rules: {
'mark-used-page-properties': require(`../ast-linter/rules/mark-used-page-properties`)
}, log, callback: (linter) => {
}, log, partialVerificationCache, callback: (linter) => {
linter.usedPageProperties.forEach((variable) => {
result.pageBuilderProperties.add(variable);
});
Expand All @@ -149,14 +164,14 @@ const ruleImplementations = {
},
'GS110-NO-UNKNOWN-PAGE-BUILDER-USAGE': {
isEnabled: true,
eachFile: ({file, theme, log}) => {
eachFile: ({file, theme, log, partialVerificationCache}) => {
const templateTest = file.file.match(/(?<!partials\/.+?)\.hbs$/);

if (templateTest) {
parseWithAST({
file, theme, rules: {
'no-unknown-page-properties': require(`../ast-linter/rules/lint-no-unknown-page-properties`)
}, log, callback: () => {}
}, log, partialVerificationCache, callback: () => {}
});
}
}
Expand Down

0 comments on commit f57ab07

Please sign in to comment.