Skip to content

fix(webpack): fix css sourceMap option #3272

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

Closed
wants to merge 2 commits into from
Closed

fix(webpack): fix css sourceMap option #3272

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
050e480
fix(webpack): fix css sourceMap option
mikejpeters Nov 24, 2016
e3c5ac5
Merge branch 'master' into fix-css-sourcemaps
mikejpeters Nov 24, 2016
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
8 changes: 7 additions & 1 deletion packages/angular-cli/models/webpack-build-common.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,13 @@ export function getWebpackCommonConfig(
new webpack.LoaderOptionsPlugin({
test: /\.(css|scss|sass|less|styl)$/,
options: {
postcss: [ autoprefixer() ]
postcss: [ autoprefixer() ],
// workaround for https://github.com/webpack/css-loader/issues/340
context: path.resolve(__dirname, './'),
Copy link
Contributor

Choose a reason for hiding this comment

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

Are you sure you want to use __dirname here?

Copy link
Author

Choose a reason for hiding this comment

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

It's the same value passed to context earlier in this file.

Copy link

Choose a reason for hiding this comment

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

@hansl Solving this bug is quite important IMHO. Could you please take a closer look and propose an alternative if using __dirname is not acceptable?
I've tried the fix and it and works fine (at last).
Thanks a lot!

Copy link
Contributor

Choose a reason for hiding this comment

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

Alternative: project root.

The reason is the __dirname is the path to this file, but it might actually not be in the same directory than the project (for example, when you npm link the CLI). I think the correct solution is to use the project root.

Note that tutorial use __dirname because their webpack.config.js are at the root of their project. So in their case it's the valid thing to do.

Also, path.resolve(__dirname, '.') does nothing; just put the path in there.

Copy link
Author

@mikejpeters mikejpeters Dec 3, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

What I have done is just a simple workaround for the fact that loader options plugin makes the original webpack config inaccessible to the other plugins. Probably the most correct way to apply the workaround would be to assign the entire webpack config to a var, so that the values can be re-used like this:

var webpackConfig = {
  ...
  context: path.resolve(__dirname, './'),
  ...
};

webpackConfig.plugins.push(
  new webpack.LoaderOptionsPlugin({
    test: /\.(css|scss|sass|less|styl)$/,
    options: {
      postcss: [ autoprefixer() ],
      context: webpackConfig.context, // workaround
      output: {
        path: webpackConfig.output.path // workaround
      }
    }
  })
});

return webpackConfig;

I did not do this simply because I wanted to keep the changes I make in this merge request as simple as possible. That's why I copied the exact config values from higher up in this same file. If you want those values changed I think it should be a separate merge request.

See L41 and L44

// workaround for https://github.com/jtangelder/sass-loader/issues/298
output: {
path: path.resolve(projectRoot, appConfig.outDir)
}
},
}),
],
Expand Down
8 changes: 4 additions & 4 deletions packages/angular-cli/models/webpack-build-development.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ export const getWebpackDevConfigPartial = function(projectRoot: string, appConfi
const styles = appConfig.styles
? appConfig.styles.map((style: string) => path.resolve(appRoot, style))
: [];
const cssLoaders = ['style-loader', 'css-loader?sourcemap', 'postcss-loader'];
const cssLoaders = ['style-loader', 'css-loader?sourceMap', 'postcss-loader'];
return {
output: {
path: path.resolve(projectRoot, appConfig.outDir),
Expand All @@ -23,15 +23,15 @@ export const getWebpackDevConfigPartial = function(projectRoot: string, appConfi
}, {
include: styles,
test: /\.styl$/,
loaders: [...cssLoaders, 'stylus-loader?sourcemap']
loaders: [...cssLoaders, 'stylus-loader?sourceMap']
}, {
include: styles,
test: /\.less$/,
loaders: [...cssLoaders, 'less-loader?sourcemap']
loaders: [...cssLoaders, 'less-loader?sourceMap']
}, {
include: styles,
test: /\.scss$|\.sass$/,
loaders: [...cssLoaders, 'sass-loader?sourcemap']
loaders: [...cssLoaders, 'sass-loader?sourceMap']
},
]
}
Expand Down
16 changes: 11 additions & 5 deletions packages/angular-cli/models/webpack-build-production.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ export const getWebpackProdConfigPartial = function(projectRoot: string, appConf
const styles = appConfig.styles
? appConfig.styles.map((style: string) => path.resolve(appRoot, style))
: [];
const cssLoaders = ['css-loader?sourcemap&minimize', 'postcss-loader'];
const cssLoaders = ['css-loader?sourceMap&minimize', 'postcss-loader'];
return {
output: {
path: path.resolve(projectRoot, appConfig.outDir),
Expand All @@ -37,15 +37,15 @@ export const getWebpackProdConfigPartial = function(projectRoot: string, appConf
}, {
include: styles,
test: /\.styl$/,
loaders: ExtractTextPlugin.extract([...cssLoaders, 'stylus-loader?sourcemap'])
loaders: ExtractTextPlugin.extract([...cssLoaders, 'stylus-loader?sourceMap'])
}, {
include: styles,
test: /\.less$/,
loaders: ExtractTextPlugin.extract([...cssLoaders, 'less-loader?sourcemap'])
loaders: ExtractTextPlugin.extract([...cssLoaders, 'less-loader?sourceMap'])
}, {
include: styles,
test: /\.scss$|\.sass$/,
loaders: ExtractTextPlugin.extract([...cssLoaders, 'sass-loader?sourcemap'])
loaders: ExtractTextPlugin.extract([...cssLoaders, 'sass-loader?sourceMap'])
},
]
},
Expand All @@ -71,7 +71,13 @@ export const getWebpackProdConfigPartial = function(projectRoot: string, appConf
options: {
postcss: [
require('postcss-discard-comments')
]
],
// workaround for https://github.com/webpack/css-loader/issues/340
context: path.resolve(__dirname, './'),
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as above.

Copy link
Author

Choose a reason for hiding this comment

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

See my other comment please

// workaround for https://github.com/jtangelder/sass-loader/issues/298
output: {
path: path.resolve(projectRoot, appConfig.outDir)
}
}
})
]
Expand Down