Skip to content

Commit

Permalink
feat: STRF-9749 Update Stencil Cli to use node-sass latest by default
Browse files Browse the repository at this point in the history
  • Loading branch information
jairo-bc committed Apr 26, 2022
1 parent 6a4167f commit 8eb51c5
Show file tree
Hide file tree
Showing 30 changed files with 458 additions and 17 deletions.
4 changes: 2 additions & 2 deletions .github/workflows/pull_request_review.yml
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,10 @@ jobs:

steps:
- name: Checkout code
uses: actions/checkout@v2
uses: actions/checkout@v3

- name: Use Node.js ${{ matrix.node }}
uses: actions/setup-node@v2-beta
uses: actions/setup-node@v3
with:
node-version: ${{ matrix.node }}

Expand Down
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,9 +1,14 @@
### Draft

- feat: strf-9749 Update Stencil Cli to use node-sass latest by default ([x](https:/bigcommerce/stencil-cli/pull/x))
New option:` --use-old-node-sass-fork` to bypass error warning (in case it exist) using latest node-sass version.
Soon, it node sass fork will be deprecated and it won't be possible to compile with it.

- feat: STRF-9757 Introduced "stencil debug" ([918](https:/bigcommerce/stencil-cli/pull/918))

Available options: --output [filename] (-o)
If not provided, will be logged to std

- feat: STRF-9741 Verbose network requests logging in Stencil CLI by default ([x](https:/bigcommerce/stencil-cli/pull/x))
Introduced `--no-verbose` option on all commands to supress verbose network requests logging.

Expand Down
10 changes: 10 additions & 0 deletions lib/StencilCLISettings.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ class StencilCLISettings {
constructor() {
// The setting enables/disables verbose network requests logging
this.versboseNetworkLogging = true;
// Enables usage of node sass fork for testing purposes
this.oldNodeSassFork = false;
}

setVerbose(flag) {
Expand All @@ -11,6 +13,14 @@ class StencilCLISettings {
isVerbose() {
return this.versboseNetworkLogging === true;
}

useOldNodeSassFork(flag) {
this.oldNodeSassFork = flag;
}

isOldNodeSassForkEnabled() {
return this.oldNodeSassFork;
}
}

const settings = new StencilCLISettings();
Expand Down
96 changes: 84 additions & 12 deletions lib/bundle-validator.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ const privateThemeConfigValidationSchema = require('./schemas/privateThemeConfig
const themeConfigValidationSchema = require('./schemas/themeConfig.json');
const themeValidationSchema = require('./schemas/themeSchema.json');
const cssCompiler = require('./css/compile');
const stencilCLISettings = require('./StencilCLISettings');

const VALID_IMAGE_TYPES = ['.jpg', '.jpeg', '.png', '.gif'];
const WIDTH_COMPOSED = 600;
Expand All @@ -27,6 +28,8 @@ const MAX_SIZE_COMPOSED = 1024 * 1024 * 2; // 2MB
const MAX_SIZE_MOBILE = 1024 * 1024; // 1MB
const MAX_SIZE_DESKTOP = 1024 * 1024 * 5; // 5MB

/* eslint-disable no-useless-escape */
const STYLESHEET_REGEXP = /{{\s*stylesheet\s*([\/a-zA-Z'"\.-]+)\s*}}/i;
class BundleValidator {
/**
* Run some validations to ensure that the platform will accept the theme
Expand Down Expand Up @@ -408,31 +411,100 @@ class BundleValidator {
const assetsPath = path.join(this.themePath, 'assets');
const stylesPath = path.join(this.themePath, 'assets/scss');
const rawConfig = await this.themeConfig.getRawConfig();
const compiler = rawConfig.css_compiler;

let cssFiles;
try {
const files = await fs.promises.readdir(stylesPath);
cssFiles = files.filter((file) => {
return file.slice(-(compiler.length + 1)) === `.${compiler}`;
});
} catch (e) {
throw new Error(`Error: ${e.message}, while reading files from "${stylesPath}".`.red);
}
const cssFiles = await this.getCssFiles();

for (const file of cssFiles) {
try {
const engine = stencilCLISettings.isOldNodeSassForkEnabled()
? cssCompiler.FALLBACK_SASS_ENGINE_NAME
: cssCompiler.SASS_ENGINE_NAME;
/* eslint-disable-next-line no-await-in-loop */
await cssCompiler.compile(rawConfig, assetsPath, file);
await cssCompiler.compile(rawConfig, assetsPath, file, engine);
} catch (e) {
const message = this.parseStencilStylesError(e);
if (!stencilCLISettings.isOldNodeSassForkEnabled()) {
throw new Error(
`${message}\n`.red +
`\n` +
`---------WARNING---------\n`.red +
`We are currently in the process of deprecating node-sass fork https:/bigcommerce-labs/node-sass\n` +
`Your scss files were compiled using latest node-sass version https:/sass/node-sass\n` +
`This error might indicate that your scss file is not compatible with it.\n` +
`There is still an option to compile scss file old fork by using --use-old-node-sass-fork.\n` +
`But note, that this will lead to 500 error in production in near future.\n` +
`---------WARNING---------\n`.red,
);
}
throw new Error(
`${message} while compiling css files from "${stylesPath}/${file}".`.red,
);
}
}
}

async getCssFiles() {
const templatesPath = path.join(this.themePath, 'templates');
const files = await recursiveReadDir(templatesPath);
const cssFiles = [];
for await (const file of files) {
const content = await fs.promises.readFile(file, { encoding: 'utf-8' });
const result = content.match(STYLESHEET_REGEXP);
if (result) {
// remove quotes
const fileName = result[1].slice(1, -1);

const filePath = this.tryToResolveCssFileLocation(fileName, result);
if (!cssFiles.includes(filePath)) {
// check if file exist
cssFiles.push(filePath);
}
}
}

return cssFiles;
}

// returns relative path starting from root scss/css folder
tryToResolveCssFileLocation(fileName, result) {
const possibleLocations = [
fileName,
fileName.replace('/css/', '/scss/'),
fileName.replace('/scss/', '/css/'),
fileName.replace('/css/', '/scss/').replace('.css', '.scss'),
fileName.replace('/scss/', '/css/').replace('.scss', '.css'),
];

for (const location of possibleLocations) {
const filePath = path.join(this.themePath, location);
if (fs.existsSync(filePath)) {
if (!this.isRootCssFile(location)) {
return this.getCssFileWithoutRootFolder(location);
}
const fileParts = path.parse(filePath);
return fileParts.name;
}
}

throw new Error(`Couldn't find file for this statement: ${result[0]}`.red);
}

// root folders are /assets/css /assets/scss
// so after split, there can be 3 or 4 elements in the array (depending if the leading slash is present)
isRootCssFile(location) {
return location.split('/').length <= 4;
}

getCssFileWithoutRootFolder(location) {
const locationParts = location.split('/');
if (locationParts[0] === '') {
locationParts.shift();
}
locationParts.shift();
locationParts.shift();

return locationParts.join('/');
}

parseStencilStylesError(e) {
if (e.formatted) {
return `${e.formatted.replace('Error: ', '')}: `;
Expand Down
16 changes: 16 additions & 0 deletions lib/bundle-validator.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ const { promisify } = require('util');

const ThemeConfig = require('./theme-config');
const BundleValidator = require('./bundle-validator');
const stencilCLISettings = require('./StencilCLISettings');

const themePath = path.join(process.cwd(), 'test/_mocks/themes/valid');

Expand All @@ -15,6 +16,7 @@ describe('BundleValidator', () => {

afterEach(() => {
jest.restoreAllMocks();
stencilCLISettings.useOldNodeSassFork(false);
});

it('should not run image validations for private themes', async () => {
Expand Down Expand Up @@ -126,4 +128,18 @@ describe('BundleValidator', () => {
'while parsing frontmatter',
);
});

it('should throw an error using latest node sass', async () => {
const themePath2 = path.join(
process.cwd(),
'test/_mocks/themes/invalid-scss-latest-node-sass',
);
themeConfig = ThemeConfig.getInstance(themePath2);

const validator = new BundleValidator(themePath2, themeConfig, false);

await expect(promisify(validator.validateTheme.bind(validator))()).rejects.toThrow(
'Import directives may not be used within control directives or mixins',
);
});
});
32 changes: 32 additions & 0 deletions lib/bundle-validator2.spec.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
const path = require('path');
const { promisify } = require('util');

const ThemeConfig = require('./theme-config');
const BundleValidator = require('./bundle-validator');
const stencilCLISettings = require('./StencilCLISettings');

// NOTE:
// Since stencil styles can't change node sass engine in runtime, tests are divided into 2 files
// Jest runs tests in parallel, so the node sass binaries are not conflicting and we are able to test both scenarios
//
// Skipping this test until multiple core are supported no Github Actions
describe.skip('BundleValidator', () => {
afterEach(() => {
jest.restoreAllMocks();
stencilCLISettings.useOldNodeSassFork(false);
});

it('should compile successfully with old node sass fork', async () => {
const themePath = path.join(
process.cwd(),
'test/_mocks/themes/invalid-scss-latest-node-sass',
);
const themeConfig = ThemeConfig.getInstance(themePath);
stencilCLISettings.useOldNodeSassFork(true);

const validator = new BundleValidator(themePath, themeConfig, false);

const result = await promisify(validator.validateTheme.bind(validator))();
expect(result).not.toBeNull();
});
});
6 changes: 6 additions & 0 deletions lib/cliCommon.js
Original file line number Diff line number Diff line change
Expand Up @@ -88,11 +88,17 @@ function applyCommonOptions(program) {
program
.option('-h, --host [hostname]', 'specify the api host')
.option('-nov, --no-verbose', 'supress verbose info logging', false)
.option(
'--use-old-node-sass-fork',
'use old node sass fork for scss compilation during bundling',
false,
)
.parse(process.argv);
}

function setupCLI(options) {
stencilCLISettings.setVerbose(options.verbose);
stencilCLISettings.useOldNodeSassFork(options.useOldNodeSassFork);
}

function prepareCommand(program) {
Expand Down
9 changes: 6 additions & 3 deletions lib/css/compile.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,10 @@ const { promisify } = require('util');

const cssAssembler = require('../css-assembler');

const SASS_ENGINE_NAME = 'node-sass-fork';
const FALLBACK_SASS_ENGINE_NAME = 'node-sass-fork';
const SASS_ENGINE_NAME = 'node-sass';

const compile = async (configuration, themeAssetsPath, fileName) => {
const compile = async (configuration, themeAssetsPath, fileName, engineName = SASS_ENGINE_NAME) => {
const fileParts = path.parse(fileName);
const ext = configuration.css_compiler === 'css' ? configuration.css_compiler : 'scss';
const pathToFile = path.join(fileParts.dir, `${fileParts.name}.${ext}`);
Expand All @@ -32,11 +33,13 @@ const compile = async (configuration, themeAssetsPath, fileName) => {
},
};
const stencilStyles = new StencilStyles(console);
stencilStyles.activateEngine(SASS_ENGINE_NAME);
stencilStyles.activateEngine(engineName);

return stencilStyles.compileCss('scss', params);
};

module.exports = {
compile,
SASS_ENGINE_NAME,
FALLBACK_SASS_ENGINE_NAME,
};
4 changes: 4 additions & 0 deletions lib/stencil-push.utils.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,10 @@ describe('stencil push utils', () => {
channelIds: [1, 2],
};

beforeEach(() => {
jest.spyOn(console, 'info').mockImplementation(jest.fn());
});

afterEach(() => {
jest.restoreAllMocks();
axiosMock.reset();
Expand Down
1 change: 1 addition & 0 deletions server/plugins/renderer/renderer.module.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ describe('Renderer Plugin', () => {
// Prevent littering the console
jest.spyOn(console, 'log').mockImplementation(jest.fn());
jest.spyOn(console, 'error').mockImplementation(jest.fn());
jest.spyOn(console, 'info').mockImplementation(jest.fn());

server = await Server.create(serverOptions);

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
h1 {
color: #000000;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
h1 {
color: #0000AA;
}

$font: "Arial";

@if not contains($font, "'Clear Sans', sans-serif") {
@import "test";
}
Loading

0 comments on commit 8eb51c5

Please sign in to comment.