Skip to content

Commit

Permalink
De-duplicate hoisted imports and avoid rewriting imports of built-ins (
Browse files Browse the repository at this point in the history
…#142)

* De-duplicate hoisted imports and avoid rewriting imports of built-ins
* Add changelog entry and bump version number
  • Loading branch information
amannn authored Jun 17, 2021
1 parent a29438b commit 230c4a1
Show file tree
Hide file tree
Showing 8 changed files with 75 additions and 6 deletions.
3 changes: 2 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
# Change Log
## [Unreleased]
## [2.2.2] - Unreleased
* De-duplicate hoisted imports and avoid rewriting imports of built-ins. [#142](https://github.com/shakacode/sass-resources-loader/pull/142) by [Jan Amann](https://github.com/amannn).

## [2.2.1] - 2021-04-14
* Support multi-line imports with @use syntax. Fixes: [#135](https://github.com/shakacode/sass-resources-loader/issues/135). [#140](https://github.com/shakacode/sass-resources-loader/pull/140) by [Toms Burgmanis](https://github.com/tomburgs).
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "sass-resources-loader",
"version": "2.2.1",
"version": "2.2.2",
"description": "SASS resources loader for Webpack",
"main": "lib/loader.js",
"files": [
Expand Down
17 changes: 14 additions & 3 deletions src/utils/processResources.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,23 @@ const useRegexTest = new RegExp(useRegex, 'm');
// Makes sure that only the last instance of `useRegex` variable is found
const useRegexReplace = new RegExp(`${useRegex}(?![\\s\\S]*${useRegex})`, 'gm');

const getOutput = (source, resources, { hoistUseStatements }) => {
export const getOutput = (source, resources, { hoistUseStatements }) => {
if (hoistUseStatements && useRegexTest.test(source)) {
return source.replace(
const output = source.replace(
useRegexReplace,
useStatements => `${useStatements}\n${resources}`,
(useStatements) => `${useStatements}\n${resources}`,
);

// De-duplicate identical imports
const importedResources = {};
return output.replace(new RegExp(useRegex, 'mg'), (importedResource) => {
if (importedResources[importedResource]) {
return '';
}

importedResources[importedResource] = true;
return importedResource;
});
}

return `${resources}\n${source}`;
Expand Down
6 changes: 5 additions & 1 deletion src/utils/rewritePaths.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,12 @@ export default function rewritePaths(error, file, contents, moduleContext, callb
return callback(null, contents);
}


const rewritten = contents.replace(useRegexp, (entire, importer, single, double, unquoted) => {
// Don't rewrite imports from built-ins
if (single.indexOf('sass:') === 0) {
return entire;
}

const oldUsePath = single || double || unquoted;

const absoluteUsePath = path.join(path.dirname(file), oldUsePath);
Expand Down
18 changes: 18 additions & 0 deletions test/__snapshots__/index.test.js.snap
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,20 @@ div {
"
`;

exports[`sass-resources-loader imports should not rewrite the path for built-ins with @use 1`] = `
"// Should be de-duplicated
@use 'sass:math';
$padding: #{math.div(4 / 2)}px;
div {
padding: $padding;
margin: #{math.div(4 / 2)}px;;
}
"
`;

exports[`sass-resources-loader imports should preserve import method 1`] = `
"@use 'shared/index' as secret;
@import 'shared/variables';
Expand Down Expand Up @@ -126,6 +140,10 @@ exports[`sass-resources-loader resources should parse array resources 1`] = `
@forward \\"variables\\";
@use 'sass:math';
$padding: #{math.div(4 / 2)}px;
$text-color: $ccc;
@mixin my-mixin {
Expand Down
25 changes: 25 additions & 0 deletions test/index.test.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
const path = require('path');
const webpack = require('webpack');
const { merge } = require('webpack-merge');
const { getOutput } = require('../src/utils/processResources');

const pathToLoader = require.resolve('../lib/loader.js');

Expand Down Expand Up @@ -240,5 +241,29 @@ describe('sass-resources-loader', () => {
const output = require('./output/imports2').default;
expect(output).toMatchSnapshot();
}));

it('should not rewrite the path for built-ins with @use', () => execTest('use-builtin', {
hoistUseStatements: true,
resources: [
path.resolve(__dirname, './scss/shared/_math.scss'),
],
}).then(() => {
// eslint-disable-next-line global-require
const output = require('./output/use-builtin').default;
expect(output).toMatchSnapshot();
}));
});

describe('getOutput', () => {
it('de-duplicates imports from the source when they\'re present in a resource and `hoiseUseStatements` is enabled', () => {
const result = getOutput(
"@use 'sass:math';\n@use 'sass:list';\n@use 'sass:map';\ndiv { padding: $padding; }",
"@use 'sass:math';\n$padding: 10px;",
{ hoistUseStatements: true },
);
expect(result).toBe(
"@use 'sass:math';\n@use 'sass:list';\n@use 'sass:map';\n\n$padding: 10px;\ndiv { padding: $padding; }",
);
});
});
});
3 changes: 3 additions & 0 deletions test/scss/shared/_math.scss
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
@use 'sass:math';

$padding: #{math.div(4 / 2)}px;
7 changes: 7 additions & 0 deletions test/scss/use-builtin.scss
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
// Should be de-duplicated
@use 'sass:math';

div {
padding: $padding;
margin: #{math.div(4 / 2)}px;;
}

0 comments on commit 230c4a1

Please sign in to comment.