jonkemp / inline-css

Inline css into an html file.
MIT License
429 stars 85 forks source link

Fix/vulnerabilities cheerio #115

Closed pleymor closed 2 years ago

pleymor commented 2 years ago

fixes vulnerability on cheerio

jonkemp commented 2 years ago

No need to bump the version numbers. That will happen on publish.

pleymor commented 2 years ago

✔️ done: the versions are no longer modified. @jonkemp can you double check if the tests I modified are still ok for you? Thanks! 🙏

jonkemp commented 2 years ago

Why are there .nvmrc files?

pleymor commented 2 years ago

Because the different package-lock.json have different lockfile versions: 1 or 2. These versions mean the files where compiled with different versions of npm, which are linked in nvm to the versions I mentionned in .nvmrc files.

So the presence of these files indicates to nvm (with nvm use) which version of node to be used in the given directory.

As nvm associates a version of npm to every version of node, it's a way to both document what node version is to be used in a given project and a way to load the proper version of nom to run nom install.

Le dim. 5 juin 2022, 02:23, Jonathan Kemp @.***> a écrit :

Why are there .nvmrc files?

— Reply to this email directly, view it on GitHub https://github.com/jonkemp/inline-css/pull/115#issuecomment-1146663867, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABJEW2J37TLMD7WM7D3K6ETVNONINANCNFSM5XERQNIA . You are receiving this because you authored the thread.Message ID: @.***>

jonkemp commented 2 years ago

Sorry for the delayed response on this. I don't want to include all the .nvmrc files so if you can think of another solution, that would be preferred.

pleymor commented 2 years ago

Just deleted the files :)

jonkemp commented 2 years ago

The .nvmrc file in the root is still there.

Also, why did the test output change?

jonkemp commented 2 years ago

Please use js-beautify for the unit test comparisons. Thanks.

pleymor commented 2 years ago

Hello @jonkemp, I'm done removing the last .nvmrc (sorry for the omission).

in unit tests, html is compared, not js. Do you want I try html-beautify to replace this home made function?

const expected = String(fs.readFileSync(expectedPath)).replace(/(\r\n|\n|\r)/gm, "");
html.replace(/(\r\n|\n|\r)/gm, "").should.be.equal(expected);
jonkemp commented 2 years ago

const beautify = require('js-beautify').html; will accomplish the same thing.

pleymor commented 2 years ago

@jonkemp beautify is now used in tests :heavy_check_mark:

pleymor commented 2 years ago

Lint error fixed. Note that there are 3 lint warnings left due to complexity, that could be fixed quite easily in a dedicated PR

coveralls commented 2 years ago

Coverage Status

Coverage increased (+0.02%) to 95.918% when pulling c44f378efaf5eb3afa0f76a550d0cc68d80ddc6a on pleymor:fix/vulnerabilities-cheerio into 21fb57e4971afddaf55cb5595298ad8a33472683 on jonkemp:master.

GerardSetho commented 2 years ago

Thanks @pleymor for the effort! @jonkemp Is this ok to merge?

GerardSetho commented 2 years ago

Thank you @jonkemp ! Any updates on when this will be published to NPM?

jonkemp commented 2 years ago

Tests are failing locally so as soon as I can get those passing.