Skip to content

Add resolve options for imports from css files. #474

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
weaverryan merged 6 commits into from
Mar 29, 2019
Merged

Add resolve options for imports from css files. #474

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
b54ec7b
Specify resolve option to be used with the sass-loader. This allows @…
Dec 20, 2018
0f9657f
Specify resolve option to be used with the css-loader. This allows @i…
Dec 20, 2018
b9e42be
Add functional test for sass imports via package.json sass property
Jan 7, 2019
382bc5f
Add functional test for css imports via package.json style property
Jan 7, 2019
9525e7a
Merge branch 'master' into resolve
weaverryan Mar 29, 2019
7895f63
fixing CS
weaverryan Mar 29, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions fixtures/css/sass_package_import.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
// There seems to be an issue with the sass-loader and prefixing
// aliased directories with a tilde. The following line is how
// this import should look and works for node_modules.
// @import '~lib/test-pkg';

// Importing without the tilde seems to work for webpack aliases
@import 'lib/test-pkg';
Copy link
Member

@weaverryan weaverryan Feb 2, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you walk me through this process for how this is resolved? I don't quite see the full picture yet:

  1. Webpack sees that we're loading the lib/test-pkg module
  2. But, it doesn't know yet (right?) if this is loading a JS file (silly example, but technically correct), a JSON file, a font file or a CSS file. So, how does it know to look at the resolve key from the css rule?

Copy link
Author

@deAtog deAtog Feb 6, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When the sass-loader encounters 'lib/test-pkg' it asks Webpack to perform the resolution of this import and it does so using the process outlined in the Resolve configuration. Webpack will notice that lib is an alias to ./lib and that test-pkg is a directory under it which contains a package.json file. To determine which file to import, Webpack evaluates each value in the mainFields resolve option in order to determine if it has been specified in the package.json file. If the mainFields resolve option is not specified, Webpack defaults to browser, module, and then main when the target property is set to web,webworker, or is unspecified. If none of these properties exist in the package.json file, then Webpack will revert to using extension based resolution. In this case since 'lib/text-pkg' resolves to a directory, Webpack will look for a file in this directory using the names specified in mainFiles resolve option having an extension of one of the values specified in the extensions resolve option. The mainFiles resolve option defaults to index and the extensions resolve options defaults to ['.wasm', '.mjs', '.js', '.json']. With the options below specified, Webpack will instead look for index.scss, index.sass, and then index.css.

1 change: 1 addition & 0 deletions fixtures/css/style_package_import.css
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
@import '~lib/test-pkg';
5 changes: 5 additions & 0 deletions fixtures/lib/test-pkg/css/sass_entry.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
$content:'Sass entrypoint';

body {
content:$content;
}
3 changes: 3 additions & 0 deletions fixtures/lib/test-pkg/css/style_entry.css
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
body {
content:'Style entrypoint';
}
3 changes: 3 additions & 0 deletions fixtures/lib/test-pkg/js/javascript_entry.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
"use strict";

document.write("JavaScript entrypoint");
11 changes: 11 additions & 0 deletions fixtures/lib/test-pkg/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
{
"name":"@symfony/webpack-encore-test-pkg",
"description": "This is a test package for use by functional tests which use packages.",
"author": {
"name": "David Ellingsworth",
"email": "[email protected]"
},
"main":"js/javascript_entry.js",
"style":"css/style_entry.css",
"sass":"css/sass_entry.scss"
}
8 changes: 8 additions & 0 deletions lib/config-generator.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -234,6 +234,10 @@ class ConfigGenerator {
use: babelLoaderUtil.getLoaders(this.webpackConfig)
}),
applyRuleConfigurationCallback('css', {
resolve: {
mainFields: ['style', 'main'],
extensions: ['.css'],
},
test: /\.css$/,
oneOf: [
{
Expand Down Expand Up @@ -311,6 +315,10 @@ class ConfigGenerator {

if (this.webpackConfig.useSassLoader) {
rules.push(applyRuleConfigurationCallback('sass', {
resolve: {
mainFields: ['sass', 'style', 'main'],
extensions: ['.scss', '.sass', '.css']
Copy link
Member

@weaverryan weaverryan Feb 2, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What exactly does this do in the process? I know what resolve.extensions does at the root level - it allows you to omit the file extension. But what about here? If I say import 'foo-bar' where foo-bar is a module, what is the purpose of the extensions?

Copy link
Author

@deAtog deAtog Feb 7, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The resolve.extensions value overrides the value at the root level. When you say @import 'foo-bar' Webpack will first check for an alias called foo-bar. If it does not find an alias for foo-bar it will check each directory specified in resolve.modules for a directory called foo-bar. Assuming it finds that directory, it will then look to see if there is a package.json file for the module. If there is, it will use the resolve.mainFields setting to determine which file to import. If there is not then it will use the resolve.mainFiles in conjunction with resolve.extentions to determine what file to look for in that directory.

If you were to say @import './foo-bar' then Webpack would look in the current directory for a file named foo-bar with an extension of .scss, .sass, or .css in that order.

Copy link
Member

@weaverryan weaverryan Mar 25, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One more question: because this is being added to the sass loader, this resolution logic applies to files being imported from within a Sass file only, right? So, for example, if I were in foo.css (the earlier import), ./foo-barwould look forfoo-bar.cssbut notfoo-bar.scss` (because that rule only lives down here).

What about less & stylus below? Do those also need these options?

Copy link
Author

@deAtog deAtog Mar 28, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, these are loader specific options. So @import "./foo-bar" in foo.css would only look for ./foo-bar.css and not foo-bar.scss.

These options can be specified for any loaders where appropriate so long as that loader uses Webpack to perform the import resolution.

},
test: /\.s[ac]ss$/,
oneOf: [
{
Expand Down
37 changes: 37 additions & 0 deletions test/functional.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2205,6 +2205,43 @@ module.exports = {
});
});

describe('Package entrypoint imports', () => {
it('Import via "sass" package property', (done) => {
const config = createWebpackConfig('web/build', 'dev');

config.setPublicPath('/build');
config.addAliases({
lib: path.resolve('./lib')
});
config.enableSassLoader();
config.addStyleEntry('sass', './css/sass_package_import.scss');

testSetup.runWebpack(config, () => {
// A successful compile is all that is needed to pass this test.
// If this test fails then the import in the above sass file
// is not loading the package's sass file.
done();
});
});

it('Import via "style" package property', (done) => {
const config = createWebpackConfig('web/build', 'dev');

config.setPublicPath('/build');
config.addAliases({
lib: path.resolve('./lib')
});
config.addStyleEntry('style', './css/style_package_import.css');

testSetup.runWebpack(config, () => {
// A successful compile is all that is needed to pass this test.
// If this test fails then the import in the above css file
// is not loading the package's style file.
done();
});
});
});

describe('CSS extraction', () => {
it('With CSS extraction enabled', (done) => {
const config = createWebpackConfig('build', 'dev');
Expand Down