-
Notifications
You must be signed in to change notification settings - Fork 82
refactor: use jsesc
to escape special characters
#62
base: master
Are you sure you want to change the base?
Conversation
Similar to webpack-contrib/raw-loader#36 we can use jsesc here to deal with special characters in general.
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.
Do we really need it ? Why?
.replace(/\u2028/g, '\\u2028') | ||
.replace(/\u2029/g, '\\u2029'); | ||
value = JSON.stringify(value); | ||
value = jsesc(value); |
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.
Is jsesc(value, { json: true })
a thing/require/needed here? (I'm not familiar with this module)
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.
See webpack-contrib/raw-loader#36 (comment) for reference
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.
Given webpack handles json at this point, why are we updating this here?
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.
'Native JSON Support' turned out to be mainly a push(jsonLoader)
for .json
if missing https://github.com/webpack/webpack/pull/3374/files#diff-6cff30a1f2b17edb1b17434da5a7d140
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.
lol, of course
jsesc
to escape special characters
Similar to webpack-contrib/raw-loader#36 we can use
jsesc here to deal with special characters in general.