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.

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

Jump to bottom

fix: use btoa instead Buffer #501

Merged
merged 1 commit into from
Apr 20, 2017
Merged

fix: use btoa instead Buffer #501

merged 1 commit into from
Apr 20, 2017

Conversation

alexander-akait
Copy link
Member

What kind of change does this PR introduce?

bugfix

Did you add tests for your changes?

yes

If relevant, did you update the README?

not required

Summary

#454

Does this PR introduce a breaking change?

no

Other information

not required

@codecov
Copy link

codecov bot commented Apr 20, 2017 •
edited
Loading

Codecov Report

Merging #501 into master will increase coverage by 0.28%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #501      +/-   ##
==========================================
+ Coverage    98.3%   98.59%   +0.28%     
==========================================
  Files          10       10              
  Lines         355      355              
  Branches       80       80              
==========================================
+ Hits          349      350       +1     
+ Misses          6        5       -1
Impacted Files Coverage Δ
lib/css-base.js 97.5% <100%> (+2.5%) ⬆️

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 6ee2fc6...8b6593a. Read the comment docs.

michael-ciniawsky
michael-ciniawsky approved these changes Apr 20, 2017
View reviewed changes
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.

Thy very much 😛

@michael-ciniawsky michael-ciniawsky changed the title Fixed: use btoa instead Buffer. fix: use btoa instead Buffer Apr 20, 2017
@michael-ciniawsky michael-ciniawsky self-assigned this Apr 20, 2017
@michael-ciniawsky
Copy link
Member

Fixes #454

@alexander-akait
Copy link
Member Author

@michael-ciniawsky i think to version 1 we should close all bug issues 😄

@michael-ciniawsky michael-ciniawsky merged commit fbb0714 into webpack-contrib:master Apr 20, 2017
@alexander-akait alexander-akait deleted the issue-454 branch April 20, 2017 11:41
@michael-ciniawsky michael-ciniawsky mentioned this pull request Apr 20, 2017
Closed
@alexander-akait
Copy link
Member Author

@michael-ciniawsky and be good release one patch version before v1 it is allow catch all regression errors, if of course they will be 😄

fahad19 added a commit to fahad19/css-loader that referenced this pull request Apr 24, 2017
@fahad19
Merge branch 'master' into externals
145e9ac 
* master:
  fix: case insensitivity of @import (webpack-contrib#514)
  chore: update comment (webpack-contrib#515)
  docs(README): improve importLoaders section and example (webpack-contrib#512)
  docs(README): fix link (webpack-contrib#508)
  docs(README): fix typo in example (webpack-contrib#507)
  docs(README): fix typo in maintainers link (webpack-contrib#505)
  fix: imported variables are replaced in exports if followed by a comma (webpack-contrib#504)
  docs(README): standardize (webpack-contrib#503)
  test: `charset` directive (webpack-contrib#502)
  fix: url with a trailing space is now handled correctly (webpack-contrib#494)
  fix: use `btoa` instead `Buffer` (webpack-contrib#501)
  test: add test for escaped characters (webpack-contrib#493)
  test: add tests for encoded svg data uri (webpack-contrib#492)
  test: add tests when css contain data uri and source maps are enabled (webpack-contrib#491)
  fix: loader now correctly handles `url` with space(s) (webpack-contrib#495)
eliperelman referenced this pull request in neutrinojs/neutrino Jul 10, 2018
@renovate @eliperelman
Update dependency css-loader to v1 (#988)
c4d0f30 
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).
webdiscus
webdiscus reviewed Feb 21, 2022
View reviewed changes
test/cssBaseTest.js
@@ -46,4 +64,17 @@ describe("css-base", function() {
}]);
m.toString().should.be.eql("body { a: 1; }\n/*# sourceURL=webpack://./path/to/test.scss */\n/*# sourceMappingURL=data:application/json;charset=utf-8;base64,eyJmaWxlIjoidGVzdC5zY3NzIiwic291cmNlcyI6WyIuL3BhdGgvdG8vdGVzdC5zY3NzIl0sIm1hcHBpbmdzIjoiQUFBQTsiLCJzb3VyY2VSb290Ijoid2VicGFjazovLyJ9 */");
});
it("should toString without source mapping if btoa not avalibale", function() {
Copy link

Choose a reason for hiding this comment

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

Why CSS should be extracted without source mapping if btoa not avalibale?
This is BREAKING CHANGE. In node <= 14 the CSS has not the inline source map.
If btoa not avalibale then shoul be used the Buffer.from(string).toString('base64') instead of btoa(string).

Copy link
Member Author

Choose a reason for hiding this comment

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

You can't use Buffer.from in browser

Copy link

Choose a reason for hiding this comment

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

ok, I understand.

P.S: I use the css-loader runtime in a webpack plugin to generate inline source-map. In node 16 it works, but not in older node.

Copy link
Member Author

Choose a reason for hiding this comment

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

Because it was not design for this, you can send a PR and improve it

alexander-akait
alexander-akait commented Feb 21, 2022
View reviewed changes
lib/css-base.js
@@ -54,7 +54,7 @@ function cssWithMappingToString(item, useSourceMap) {
return content;
}

if (useSourceMap) {
if (useSourceMap && typeof btoa === 'function') {
Copy link
Member Author

Choose a reason for hiding this comment

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

You can't use Buffer.from in browser

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@webdiscus webdiscus webdiscus left review comments

@michael-ciniawsky michael-ciniawsky michael-ciniawsky approved these changes

Assignees

@michael-ciniawsky michael-ciniawsky

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@alexander-akait @michael-ciniawsky @webdiscus