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

fix: add support for aliases starting with / (options.alias) #597

Merged
merged 2 commits into from
Aug 30, 2017

Conversation

atinux
Copy link
Contributor

@atinux atinux commented Aug 22, 2017

What kind of change does this PR introduce?

Improvement in alias handling

Did you add tests for your changes?

Of course

Summary

Hi :)

Thank you for this great webpack loader. Actually, we are using it a lot into nuxt.js.

We have an issue with the css-loader used into the vue files, we have a global webpack alias named ~ and it's used a lot to require file this way: import '~/assets/my-file.vue'

Actually, we are using css-loader with this configuration:

{
    loader: 'css-loader',
    options: {
      minimize: true,
      importLoaders: 1,
      sourceMap: true,
      alias: {
        '/static': join(this.options.srcDir, 'static'),
        '/assets': join(this.options.srcDir, 'assets')
      }
   }
}

So our users can do this into their CSS:

.logo {
  background: url('~/static/logo.png');
}
.logo-2 {
  background: url('/logo.png');
}

Before this PR, .logo background url is transformed to /static/logo.png and considered as an url not processed (even if it's starting by an alias), .logo-2 is not processed and that's perfect.

Aftet this PR, .logo is transformed as well and considered to be a relative path (since starting by a known alias) and .logo-2 is not changed 👍

Does this PR introduce a breaking change?

No breaking change

Related issue to nuxt.js repo: nuxt/nuxt#1435

@jsf-clabot
Copy link

jsf-clabot commented Aug 22, 2017

CLA assistant check
All committers have signed the CLA.

Copy link
Member

@alexander-akait alexander-akait left a comment

Choose a reason for hiding this comment

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

Good work! Thanks!

@atinux
Copy link
Contributor Author

atinux commented Aug 23, 2017

I believe it could be a minor change or patch since it's only a bug fix :)

@yuchonghua
Copy link

Very good support, probably when the merger can be released?
/ping @evilebottnawi

@meiyuanbo
Copy link

probably when the merger can be released?

@alexander-akait
Copy link
Member

@michael-ciniawsky @d3viant0ne friendly ping, need review, thanks!

@michael-ciniawsky michael-ciniawsky changed the title Add support for aliases starting with / fix: add support for aliases starting with / (options.alias) Aug 24, 2017
@@ -83,6 +83,11 @@ var parserPlugin = postcss.plugin("css-loader-parser", function(options) {
exports[exportName] = replaceImportsInString(exports[exportName]);
});

function startByAlias(url) {
Copy link
Member

Choose a reason for hiding this comment

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

startsWithAlias => isAlias[ed]

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I pushed a change to rename it to isAlias

@michael-ciniawsky michael-ciniawsky added this to the 0.28.6 milestone Aug 24, 2017
@michael-ciniawsky
Copy link
Member

Relabeled to 🏷 semver: Patch for now as I don't think this would be a breaking change, but anyone might chime in and correct me if I'm wrong 😛

@alexander-akait
Copy link
Member

@michael-ciniawsky oh, yep, it is patch, just invalid choice semver

@appinteractive
Copy link

Can someone click on merge please? 👍

@alexander-akait
Copy link
Member

@appinteractive need minimum 3 approved votes
@michael-ciniawsky @d3viant0ne friendly ping

@HadiChen
Copy link

HadiChen commented Aug 25, 2017

Can someone click on merge please? 👍

@codecov
Copy link

codecov bot commented Aug 25, 2017

Codecov Report

Merging #597 into master will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #597      +/-   ##
==========================================
+ Coverage   98.91%   98.91%   +<.01%     
==========================================
  Files          10       10              
  Lines         367      369       +2     
  Branches       87       87              
==========================================
+ Hits          363      365       +2     
  Misses          4        4
Impacted Files Coverage Δ
lib/loader.js 100% <ø> (ø) ⬆️
lib/processCss.js 98.67% <100%> (+0.01%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e16bdeb...4367442. Read the comment docs.

Copy link
Member

@michael-ciniawsky michael-ciniawsky left a comment

Choose a reason for hiding this comment

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

@atinux Thx

@joshwiens joshwiens merged commit 63567f2 into webpack-contrib:master Aug 30, 2017
@michael-ciniawsky michael-ciniawsky modified the milestone: 0.28.6 Aug 30, 2017
@liambuchanan
Copy link
Contributor

Haven't done much digging yet as this is a sub-dependency for us, but it appears merging has introduced a bug as processCss is not being instantiated with a resolve method here https://github.com/webpack-contrib/css-loader/blob/master/lib/localsLoader.js#L18

eliperelman referenced this pull request in neutrinojs/neutrino Jul 10, 2018
This Pull Request updates dependency [css-loader](https://github.com/webpack-contrib/css-loader) from `^0.28.11` to `^1.0.0`



<details>
<summary>Release Notes</summary>

### [`v1.0.0`](https://github.com/webpack-contrib/css-loader/blob/master/CHANGELOG.md#&#8203;100httpsgithubcomwebpack-contribcss-loadercomparev02811v100-2018-07-06)
[Compare Source](webpack-contrib/css-loader@v0.28.11...v1.0.0)
##### BREAKING CHANGES

* remove `minimize` option, use [`postcss-loader`](https://github.com/postcss/postcss-loader) with [`cssnano`](https://github.com/cssnano/cssnano) or use [`optimize-cssnano-plugin`](https://github.com/intervolga/optimize-cssnano-plugin) plugin
* remove `module` option, use `modules` option instead
* remove `camelcase` option, use `camelCase` option instead
* remove `root` option, use [`postcss-loader`](https://github.com/postcss/postcss-loader) with [`postcss-url`](https://github.com/postcss/postcss-url) plugin
* remove `alias` option, use [`resolve.alias`](https://webpack.js.org/configuration/resolve/) feature or use [`postcss-loader`](https://github.com/postcss/postcss-loader) with [`postcss-url`](https://github.com/postcss/postcss-url) plugin
* update `postcss` to `6` version
* minimum require `nodejs` version is `6.9`
* minimum require `webpack` version is `4`
#### [0.28.11](webpack-contrib/css-loader@v0.28.10...v0.28.11) (2018-03-16)
##### Bug Fixes

* **lib/processCss:** don't check `mode` for `url` handling (`options.modules`) ([#&#8203;698](`https://github.com/webpack-contrib/css-loader/issues/698`)) ([c788450](webpack-contrib/css-loader@c788450))
#### [0.28.10](webpack-contrib/css-loader@v0.28.9...v0.28.10) (2018-02-22)
##### Bug Fixes

* **getLocalIdent:** add `rootContext` support (`webpack >= v4.0.0`) ([#&#8203;681](`https://github.com/webpack-contrib/css-loader/issues/681`)) ([9f876d2](webpack-contrib/css-loader@9f876d2))
#### [0.28.9](webpack-contrib/css-loader@v0.28.8...v0.28.9) (2018-01-17)
##### Bug Fixes

* ignore invalid URLs (`url()`) ([#&#8203;663](`https://github.com/webpack-contrib/css-loader/issues/663`)) ([d1d8221](webpack-contrib/css-loader@d1d8221))
#### [0.28.8](webpack-contrib/css-loader@v0.28.7...v0.28.8) (2018-01-05)
##### Bug Fixes

* **loader:** correctly check if source map is `undefined` ([#&#8203;641](`https://github.com/webpack-contrib/css-loader/issues/641`)) ([0dccfa9](webpack-contrib/css-loader@0dccfa9))
* proper URL escaping and wrapping (`url()`) ([#&#8203;627](`https://github.com/webpack-contrib/css-loader/issues/627`)) ([8897d44](webpack-contrib/css-loader@8897d44))
#### [0.28.7](webpack-contrib/css-loader@v0.28.6...v0.28.7) (2017-08-30)
##### Bug Fixes

* pass resolver to `localsLoader` (`options.alias`)  ([#&#8203;601](`https://github.com/webpack/css-loader/issues/601`)) ([8f1b57c](webpack-contrib/css-loader@8f1b57c))
#### [0.28.6](webpack-contrib/css-loader@v0.28.5...v0.28.6) (2017-08-30)
##### Bug Fixes

* add support for aliases starting with `/` (`options.alias`) ([#&#8203;597](`https://github.com/webpack/css-loader/issues/597`)) ([63567f2](webpack-contrib/css-loader@63567f2))
#### [0.28.5](webpack-contrib/css-loader@v0.28.4...v0.28.5) (2017-08-17)
##### Bug Fixes

* match mutliple dashes (`options.camelCase`) ([#&#8203;556](`https://github.com/webpack/css-loader/issues/556`)) ([1fee601](webpack-contrib/css-loader@1fee601))
* stricter `[@import]` tolerance ([#&#8203;593](`https://github.com/webpack/css-loader/issues/593`)) ([2e4ec09](webpack-contrib/css-loader@2e4ec09))
#### [0.28.4](webpack-contrib/css-loader@v0.28.3...v0.28.4) (2017-05-30)
##### Bug Fixes

* preserve leading underscore in class names ([#&#8203;543](`https://github.com/webpack/css-loader/issues/543`)) ([f6673c8](webpack-contrib/css-loader@f6673c8))
#### [0.28.3](webpack-contrib/css-loader@v0.28.2...v0.28.3) (2017-05-25)
##### Bug Fixes

* correct plugin order for CSS Modules ([#&#8203;534](`https://github.com/webpack/css-loader/issues/534`)) ([b90f492](webpack-contrib/css-loader@b90f492))
#### [0.28.2](webpack-contrib/css-loader@v0.28.1...v0.28.2) (2017-05-22)
##### Bug Fixes

* source maps path on `windows` ([#&#8203;532](`https://github.com/webpack/css-loader/issues/532`)) ([c3d0d91](webpack-contrib/css-loader@c3d0d91))
#### [0.28.1](webpack-contrib/css-loader@v0.28.0...v0.28.1) (2017-05-02)
##### Bug Fixes

* allow to specify a full hostname as a root URL ([#&#8203;521](`https://github.com/webpack/css-loader/issues/521`)) ([06d27a1](webpack-contrib/css-loader@06d27a1))
* case insensitivity of [@&#8203;import] ([#&#8203;514](`https://github.com/webpack/css-loader/issues/514`)) ([de4356b](webpack-contrib/css-loader@de4356b))
* don't handle empty [@&#8203;import] and url() ([#&#8203;513](`https://github.com/webpack/css-loader/issues/513`)) ([868fc94](webpack-contrib/css-loader@868fc94))
* imported variables are replaced in exports if followed by a comma ([#&#8203;504](`https://github.com/webpack/css-loader/issues/504`)) ([956bad7](webpack-contrib/css-loader@956bad7))
* loader now correctly handles `url` with space(s) ([#&#8203;495](`https://github.com/webpack/css-loader/issues/495`)) ([534ea55](webpack-contrib/css-loader@534ea55))
* url with a trailing space is now handled correctly ([#&#8203;494](`https://github.com/webpack/css-loader/issues/494`)) ([e1ec4f2](webpack-contrib/css-loader@e1ec4f2))
* use `btoa` instead `Buffer` ([#&#8203;501](`https://github.com/webpack/css-loader/issues/501`)) ([fbb0714](webpack-contrib/css-loader@fbb0714))
##### Performance Improvements

* generate source maps only when explicitly set ([#&#8203;478](`https://github.com/webpack/css-loader/issues/478`)) ([b8f5c8f](webpack-contrib/css-loader@b8f5c8f))

---

</details>




---

This PR has been generated by [Renovate Bot](https://renovatebot.com).
shadow-shaman0209 pushed a commit to shadow-shaman0209/arsenal-force that referenced this pull request Oct 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants