-
-
Notifications
You must be signed in to change notification settings - Fork 158
feat: add encoding & generator options #210
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One note
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good work, one note - original issue was for supporting something line this https://github.com/bhovhannes/svg-url-loader (allow to reduce size of svg), I don't know beset solution, maybe we should to add a new option what allow to implement own custom source.toString()
, maybe the source
option and provide example with https://github.com/tigt/mini-svg-data-uri
@evilebottnawi I don't think |
README.md
Outdated
loader: 'url-loader', | ||
options: { | ||
source: (svgContentBuffer) => { | ||
return `data:image/svg;utf8,${svgContentBuffer.toString( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add example with mini-svg-data-uri
package
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@evilebottnawi I've added an example here: https://github.com/webpack-contrib/url-loader/pull/210/files/2bb2de67e75d175c1ab7b41a075307414f8b6930#diff-04c6e90faac2675aa89e2176d2eec7d8R259
this example to tell the people what is the final format of the final output to help them develop their own implementation.
src/index.js
Outdated
@@ -45,11 +46,13 @@ export default function loader(src) { | |||
const esModule = | |||
typeof options.esModule !== 'undefined' ? options.esModule : true; | |||
|
|||
const encodedData = options.generator | |||
? options.generator(src) | |||
: `data:${mimetype || ''};${encoding},${src.toString(encoding)}`; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Look here https://developer.mozilla.org/en-US/docs/Web/HTTP/Basics_of_HTTP/Data_URIs
encoding
is optinal, so
data:text/html,%3Ch1%3EHello%2C%20World!%3C%2Fh1%3E
is valid, but in our case we always insert ;
Rewrite it to "data:${mimetype || ''}${encoding? ;${encoding}
: ''},${src.toString(encoding)}`;" + test
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've added boolean
type for encoding, now we can disable it, if it's enabled it will use the default base64
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One note
@@ -48,7 +52,9 @@ export default function loader(src) { | |||
|
|||
const encodedData = options.generator | |||
? options.generator(src) | |||
: `data:${mimetype || ''};${encoding},${src.toString(encoding)}`; | |||
: `data:${mimetype || ''}${encoding ? `;${encoding}` : ''},${ | |||
encoding ? src.toString(encoding) : src.toString() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can be simplified to src.toString(encoding)
, when encoding
is undefined it will be as src.toString()
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
encoding
here is never undefined
it's base64
if it was already undefined or true or a string
value or false
and src.toString(false)
will cause a bug.
src/options.json
Outdated
@@ -7,7 +7,7 @@ | |||
}, | |||
"encoding": { | |||
"description": "Specify the encoding which the file will be in-lined with.", | |||
"type": "string", | |||
"type": ["boolean", "string"], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oneOf
- https://ajv.js.org/keywords.html#oneof
"oneOf": [ { "enum" : ["value", "value", "value"] }, { "type": "boolean" } ]
Thanks! |
This PR contains a:
Motivation / Use-Case
Fix: #25
Breaking Changes
No
Additional Info