Add unit test and linter #4
Conversation
index.js
Outdated
|
|
||
| return 'module.exports = Buffer.from("' + content.toString('base64') + '", "base64")'; | ||
| }; | ||
| return `module.exports = Buffer.from("${content}")` |
There was a problem hiding this comment.
I believe content needs to be encoded as being binary data, it may contain characters which would break the naïve approach here, e.g. ", \n, \0, etc.
There was a problem hiding this comment.
The Webpack docs seem to be wrong, what Webpack passes your loader is actually a Buffer object: this is why we can call .toString('base64') on it, for example, which wouldn't work with a string. In general, it wouldn't make sense to load an arbitrary file as a string so it has to work this way.
When you write ${content} though, you are asking this Buffer to convert itself to a string. Buffer is smart enough to escape control characters to escape sequences (e.g. \u0000) but as @nhardy says, it doesn't know that you are pasting that string between double quotes so it will not escape ", \ or \n, breaking the generated source. Also, Buffer will try to interpret the file's contents as UTF-8 and make a mess of things if the file is anything else.
What exactly is the problem you are encountering with the test?
There was a problem hiding this comment.
Aah, the trick is that because you have module.exports.raw = true; in the loader, Webpack calls it with a Buffer and not a string. So the docs are probably accurate for the general case, but I can't find documentation for that raw flag.
There was a problem hiding this comment.
Ahh, makes sense. Thank you!
I've pushed a new commit, please have a look again 🙏
index.spec.js
Outdated
|
|
||
| describe('buffer-loader', () => { | ||
| it('returns a buffer with the content', () => { | ||
| const content = 'some random content' |
There was a problem hiding this comment.
If you want this to reflect the way Webpack calls your loader, you would have to write this as:
const content = Buffer.from('some random content', 'utf-8');
| "jest": { | ||
| "testEnvironment": "node" | ||
| }, | ||
| "standard": { |
There was a problem hiding this comment.
standard is nice but semistandard is better 😉
Hey @jonathanperret and @nhardy,
While I was adding a unit test to this loader, doing
buffer.toString()ofsome random contentreturned�����ݢg(�קandbuffer.toString('base64')returnedsomerandomcontenwhich also seems incorrect.As according to the webpack docs, "the loader is called with only one parameter -- a string containing the content of the resource file", I tried changing it to
module.exports = Buffer.from("${content}").Does this look good to you? Or am I missing something?