Skip to content

Commit

Permalink
feat(sass): upgrade sass to v1.58.3 (#229)
Browse files Browse the repository at this point in the history
this commit upgrades sass to v1.58.3 from v1.42.0.

between the aforementioned versions of sass, several changes occurred in
the library. some of which are accounted for in this commit, others are
withheld for another day.

as of sass v1.45.0, the library produces/exports its own types,
leading to the removal of the third party `@types` library.

this commit updates how @stencil/sass bundles sass. the shape of the
output code has changed such that sass' entrypoint must call a load()
function to add `render` (which performs the compilation step) to the
prototype. the rollup plugin that performs this action has been updated
to perform this call to `load()`, then subsequently export `render`.
failing to do so will result in `render` being `undefined`.

the ways in which libraries such as chokidar, readline, and immutable
are used have changes as well (immutable has been added to sass since it
was last upgraded). the rollup configuration to remove these from the
bundle is believed to be valid, so long as we do not use the new sass
javascript api (more on that coming).

sass v1.45.0 introduced a new javascript api. it is intended to the new
api to supersede the old one (that this library is currently using). as
a result, many of the types that we were exporting from the sass library
(and its typings) have new names. often, these names are prefixed with
'Legacy', to denote that the type is deprecated. these types had little
change to their constituents, and have been called out as such when they
did.

this commit does not attempt to move the project off the legacy
javascript sass api at this time. this was done for a few reasons:
1. it would introduce additional churn to the commit/pull request.
   although this migration itself will likely be a breaking change, we
   attempt a large upgrade here, and would like to keep the scope
   minimized.
2. the stencil team must revisit the value proposition of bundling sass.
   the current bundling configuration is fragile, and can break at any
   time. this decision will affect how permissive we are in allowing
   different versions of sass to be used with this project.

additional jsdoc/typings for configuration (JS) files have been added to
improve understanding of the rollup configuration/type safety where
possible.

BREAKING CHANGE: Sass often has 'potential breaking changes' in minor
verisons of the library. For this upgrade, the following versions of
sass may include a breaking change:
- [1.57.0](https://github.com/sass/dart-sass/releases/tag/1.57.0)
- [1.56.0](https://github.com/sass/dart-sass/releases/tag/1.56.0)
- [1.55.0](https://github.com/sass/dart-sass/releases/tag/1.55.0)
- [1.51.0](https://github.com/sass/dart-sass/releases/tag/1.51.0)
- [1.49.10](https://github.com/sass/dart-sass/releases/tag/1.49.10)
- [1.48.0](https://github.com/sass/dart-sass/releases/tag/1.48.0)
Users should be review the linked changelogs and test their libraries
thoroughly. Although this upgrade may not necessarily break for
consumers, it is designated as such out of caution.
  • Loading branch information
rwaskiewicz authored Feb 20, 2023
1 parent dddbf95 commit b5a3d12
Show file tree
Hide file tree
Showing 8 changed files with 134 additions and 58 deletions.
74 changes: 43 additions & 31 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 2 additions & 3 deletions package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "@stencil/sass",
"version": "2.0.3",
"version": "3.0.0-dev.2023.2.16.0",
"license": "MIT",
"main": "dist/index.js",
"module": "dist/index.mjs",
Expand Down Expand Up @@ -31,13 +31,12 @@
"@stencil/core": "^3.0.0",
"@types/jest": "^26.0.15",
"@types/node": "^16.11.48",
"@types/sass": "^1.16.0",
"jest": "^26.6.3",
"np": "^7.0.0",
"prettier": "^2.2.1",
"rimraf": "^4.0.4",
"rollup": "^3.5.1",
"sass": "^1.29.0",
"sass": "^1.58.3",
"terser": "^5.3.8",
"ts-jest": "^26.4.4",
"typescript": "^4.9.4"
Expand Down
6 changes: 6 additions & 0 deletions rollup.config.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,16 @@ import { createRequire } from 'node:module';
import pluginSass from './rollup.plugin.sass.mjs';
import rollupResolve from '@rollup/plugin-node-resolve';

// require `package.json` in order to use its 'main' and 'module' fields to tell rollup where to output the generated
// bundles
const require = createRequire(import.meta.url);
const pkg = require('./package.json');

/**
* Generate an ESM and a CJS output bundle
*/
export default {
// the input is expected to exist at this location as a result of running the typescript compiler
input: 'dist/index.js',

plugins: [
Expand Down
68 changes: 59 additions & 9 deletions rollup.plugin.sass.mjs
Original file line number Diff line number Diff line change
@@ -1,17 +1,29 @@
import { fileURLToPath } from 'node:url'
import { minify } from 'terser';

/**
* A rollup plugin for bundling Sass directly into the project
*/
export default function() {
const sassFilePath = fileURLToPath(new URL('node_modules/sass/sass.dart.js', import.meta.url));
return {
/**
* A rollup build hook for resolving the Sass implementation module.
* @param {string} id the importee exactly as it is written in an import statement in the source code
* @returns {string | undefined} the path to the Sass implementation from the root of this project
*/
resolveId(id) {
if (id === 'sass') {
return sassFilePath;
}
},
/**
* Wraps Sass to bundle it into the project
* @param {string} code the code to modify
* @param {string} id module's identifier
* @returns {string} the modified code
*/
async transform(code, id) {
// a little nudge to make it easier for
// rollup to find the cjs exports
if (id === sassFilePath) {
return await wrapSassImport(code);
}
Expand All @@ -22,6 +34,15 @@ export default function() {
};


/**
* Wraps Sass in an IIFE to make it easier for rollup to find CJS exports and minifies it.
*
* This function generates code for calling Sass' entrypoint function (`load()`) and capturing a reference to its
* `render` function.
*
* @param {string} code the Sass implementation code
* @returns {Promise<string>} the wrapped Sass code
*/
async function wrapSassImport(code) {
code = `
Expand Down Expand Up @@ -57,6 +78,7 @@ ${code};
})(Sass);
Sass.load({});
const render = Sass.render;
export { render };
`;
Expand All @@ -72,25 +94,53 @@ export { render };
'false /** NODE ENVIRONMENT **/'
);

code = removeNodeRequire(code, 'chokidar');
code = removeNodeRequire(code, 'readline');
code = removeCliPkgRequire(code, 'chokidar');
code = removeCliPkgRequire(code, 'readline');
code = removeNodeRequire(code, 'immutable');

const minified = await minify(code, { module: true });
code = minified.code;

return code
}

function removeNodeRequire(code, moduleId) {
const requireStr = `require("${moduleId}")`;
/**
* Node modules are required by node_modules/sass/sass.dart.js via `_cli_pkg_requires`.
*
* This function manually removes unneeded require statements from the source.
*
* @param {string} code the code to modify
* @param {string} moduleId the module identifier found in a require-like statement
* @returns {string} the modified code
*/
function removeCliPkgRequire(code, moduleId) {
// e.g. `self.chokidar = _cli_pkg_requires.chokidar;`
const requireStr = `self.${moduleId} = _cli_pkg_requires.${moduleId};`;
if (!code.includes(requireStr)) {
// node modules are required by sass.dart
// however this build doesn't use or need them
// so we'll manually remove it from the source
throw new Error(`cannot find "${requireStr}" in sass.dart`);
}
return code.replace(
requireStr,
'{}'
);
}

/**
* Node modules are required by node_modules/sass/sass.dart.js via `require`.
*
* This function manually removes unneeded require statements from the source.
*
* @param {string} code the code to modify
* @param {string} moduleId the module identifier found in a require-like statement
* @returns {string} the modified code
*/
function removeNodeRequire(code, moduleId) {
const requireStr = `require("${moduleId}")`;
if (!code.includes(requireStr)) {
throw new Error(`cannot find "${requireStr}" in sass.dart`);
}
return code.replace(
requireStr,
'{}'
);
}
8 changes: 6 additions & 2 deletions src/diagnostics.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { SassException } from 'sass';
import { LegacyException } from 'sass';
import * as d from './declarations';

/**
Expand All @@ -12,7 +12,11 @@ import * as d from './declarations';
* @param filePath the path of the file that led to an error being raised
* @returns the created diagnostic, or `null` if one could not be generated
*/
export function loadDiagnostic(context: d.PluginCtx, sassError: SassException, filePath: string): d.Diagnostic | null {
export function loadDiagnostic(
context: d.PluginCtx,
sassError: LegacyException,
filePath: string
): d.Diagnostic | null {
if (sassError == null || context == null) {
return null;
}
Expand Down
4 changes: 2 additions & 2 deletions src/index.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { render } from 'sass';
import { type LegacyException, type LegacyResult, render } from 'sass';
import * as d from './declarations';
import { loadDiagnostic } from './diagnostics';
import { createResultsId, getRenderOptions, usePlugin } from './util';
Expand Down Expand Up @@ -46,7 +46,7 @@ export function sass(opts: d.PluginOptions = {}): d.Plugin {
return new Promise<d.PluginTransformResults>((resolve) => {
try {
// invoke sass' compiler at this point
render(renderOpts, (err, sassResult) => {
render(renderOpts, (err: LegacyException, sassResult: LegacyResult): void => {
if (err) {
loadDiagnostic(context, err, fileName);
results.code = `/** sass error${err && err.message ? ': ' + err.message : ''} **/`;
Expand Down
Loading

0 comments on commit b5a3d12

Please sign in to comment.