search

npm / marky-markdown

npm's markdown parser
https://www.npmjs.com/package/@npmcorp/marky-markdown
405 stars 71 forks source link

Webpack support #417

Closed DimitarTachev closed 6 years ago

DimitarTachev commented 7 years ago

Wrapped the require.resolve with an eval in order to avoid getting a number instead of the file path using Webpack. Check https://github.com/webpack/webpack/issues/1554#issuecomment-158679844

bcoe commented 7 years ago

@DimitarTachev calling eval always sets off alarm bells for me; my concern would be we eventually make languages configurable, as an example, and then have an attack vector for users injecting code.

To me this feels a bit like this is a bug with webpack that should be addresses -- is it that the resolve isn't working in a browser context, or where in the build process does this error happen?

DimitarTachev commented 7 years ago

Hi @bcoe,

Thanks for getting into this. An exception is thrown runtime (node.js process) in this highlightjs method as its expecting a folder path, not a number. The problem is that in the webpack context, the resolve method returns a number. Wrapping it with an 'eval' statement, it is evaluated in the nodejs context which returns a file path and everything is working as expected.

I knew that most probably you won't merge this as "eval is evil" but I wasn't able to find any other solutions. We will just use our fork as it looks safe till the eval content is always a constant.