grunt
grunt copied to clipboard
gruntjs
String.prototype is poisoned by require('colors')
grunt requires grunt-legacy-log for console output, which requires colors as module to color the output w/ console escape characters. By requiring colors the simple way, it poisons (extends) the String.prototype.
IMHO this can cause very nasty bugs to find when running code in the context of grunt.
colors now allows using var colors = require('colors/safe'); so prototype poisoning can be mitigated.
I would be glad to do a PR for this in the grunt-legacy-log repo, however considering the states of already existing PRs, i'm afraid it won't get maintainer attention.
Could u guys form an opinion about this issue?
We've had plans to move to chalk instead of colors so it no longer pollutes the String prototype. The only issue is it would be a breaking change. So we should look into how often it is used in the Grunt ecosystem to gauge whether we could easily remove the prototype extension. Likely we'll need to display a deprecation warning first then remove the prototype extension in the next major.
@shama i c. any thoughts about just using colors/safe as advised in colors#usage section?
That way the pollution would stop however the output would stay the same.
@p1100i It would still be a breaking change as 'an error'.red would break inside user's Gruntfiles.
@shama ahh, i c. if I create a PR about the change do u think we could create a “pre-“ tagged version - published to npm - w/ your suggested version bump? or anything that would be published/merged later on by u guys? i would love to avoid using my own fork of grunt.
On 2018. May 26., at 19:15, Kyle Robinson Young [email protected] wrote:
@p1100i It would still be a breaking change as 'an error'.red would break inside user's Gruntfiles.
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.