Skip to content
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.

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

Add resolve options for imports from css files. #474

Merged
merged 6 commits into from Mar 29, 2019
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
7 changes: 7 additions & 0 deletions fixtures/css/sass_package_import.scss
@@ -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

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

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
@@ -0,0 +1 @@
@import '~lib/test-pkg';
5 changes: 5 additions & 0 deletions fixtures/lib/test-pkg/css/sass_entry.scss
@@ -0,0 +1,5 @@
$content:'Sass entrypoint';

body {
content:$content;
}
3 changes: 3 additions & 0 deletions fixtures/lib/test-pkg/css/style_entry.css
@@ -0,0 +1,3 @@
body {
content:'Style entrypoint';
}
3 changes: 3 additions & 0 deletions fixtures/lib/test-pkg/js/javascript_entry.js
@@ -0,0 +1,3 @@
"use strict";

document.write("JavaScript entrypoint");
11 changes: 11 additions & 0 deletions fixtures/lib/test-pkg/package.json
@@ -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": "david@desource.org"
},
"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
Expand Up @@ -227,6 +227,10 @@ class ConfigGenerator {
use: babelLoaderUtil.getLoaders(this.webpackConfig)
},
{
resolve: {
mainFields: ['style', 'main'],
extensions: ['.css'],
},
test: /\.css$/,
use: cssExtractLoaderUtil.prependLoaders(this.webpackConfig, cssLoaderUtil.getLoaders(this.webpackConfig))
}
Expand Down Expand Up @@ -290,6 +294,10 @@ class ConfigGenerator {

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

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

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

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

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$/,
use: cssExtractLoaderUtil.prependLoaders(this.webpackConfig, sassLoaderUtil.getLoaders(this.webpackConfig))
});
Expand Down
37 changes: 37 additions & 0 deletions test/functional.js
Expand Up @@ -1972,5 +1972,42 @@ 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();
})
});
});
});
});