marked@2.0.0 throws SyntaxError: Invalid regular expression

[wickedest]

Marked version:
2.0.0

Describe the bug
Markdown that worked previously, now errors with: SyntaxError: Invalid regular expression: /[\p{L}\p{N}]/: Invalid escape

To Reproduce

1. Install marked npm install --save marked@2.0.0
2. Create index.js:

const marked = require('marked');
const text = 'one _expectem_\ntwo _expect__em_\nthree _expect__another__em_\n';
console.log(marked(text));

3. node index.js

SyntaxError: Invalid regular expression: /[\p{L}\p{N}]/: Invalid escape
Please report this to https://github.com/markedjs/marked.
    at Tokenizer.emStrong (/home/example/node_modules/marked/src/Tokenizer.js:534:30)
    at Lexer.inlineTokens (/home/example/node_modules/marked/src/Lexer.js:418:34)
    at Lexer.inline (/home/example/node_modules/marked/src/Lexer.js:282:16)
    at Lexer.lex (/home/example/node_modules/marked/src/Lexer.js:114:10)
    at Function.lex (/home/example/node_modules/marked/src/Lexer.js:93:18)
    at marked (/home/example/node_modules/marked/src/marked.js:106:26)
    at Object.<anonymous> (/home/example/index.js:3:13)
    at Module._compile (module.js:653:30)
    at Object.Module._extensions..js (module.js:664:10)
    at Module.load (module.js:566:32)

With marked@1.2.9, this works. With marked@2.0.0, errors as above. Also, this happens on node-8. However, node-12 didn't have this issue. Possibly related to #1927.

Expected behavior
Expected previous markdown to work.

[alasdairhurst]

See nodejs/node#19052:

RegExp Unicode Property Escapes are at stage 4 and will be included in ES2018. They are available since V8 6.4 without a flag so they will be unflagged in Node.js v10. They are also available under the --harmony_regexp_property flag in Node.js v6-v9 and under the --harmony flag in Node.js v8-v9.

TLDR: #1864 introduced an additional unintentional breaking change bumping the minimum Node version from 8.16.2 to 10.0.0

https://github.com/markedjs/marked/pull/1864/files#diff-c6ae4e51c94d9a0253c6ca0afc670670f29049d741da446e6b19a22e4871821bR551

[calculuschild]

Looks like it. However if you are stuck with node v8-v9, you can run node using thr harmony flag which enables the es6 features which at the time were not stable:

node --harmony myScipt.js

Or as far back as v6 using:

node --harmony_regexp_property myScript.js

@UziTech As we drop support for internet explorer in favor of modern JS features, it looks like we will also be conflicting with old versions of Node.js in certain cases. What's our strategy here?

[alasdairhurst]

@calculuschild That's a good work around for services/utils run by their developers.

Unfortunately this would also need to be communicated to all downstream clients of updated libraries which have been updated to depend on the new version of marked(which may be used transparently) for the security fix , or the library would be forced to major bump and drop node < 10 (which is fair enough it's old, but would cause some additional churn)

Will be interested to see the strategy going forward

@UziTech why the "other package" label?

[UziTech]

@UziTech why the "other package" label?

Because the issue is in the other package. Node v8 end of life was over a year ago, so the correct solution is to update Node. Marked only supports the LTS and latest versions of Node.

If the user doesn't bother updating node why would they update other dependencies?

[alasdairhurst]

@UziTech That's fair enough, but the package.json says otherwise and gives the wrong impression:
https://github.com/markedjs/marked/blob/master/package.json#L86

If we can't guarantee any particular version of Node works with any given marked release then it defeats the point of semver. (i.e. if 10 goes EOL and a new change is subsequently added in 2.1.0 which only works on node 12)

If the user doesn't bother updating node why would they update other dependencies?

The user updating the marked version isn't necessarily the same user that's going to be getting that new version downstream. All the downstream user needs to do is be using node 8 and run npm install, expect nothing to break and suddenly it does. They're not even aware that marked is being used, and the maintainers of the package that use marked, don't know that their minimum node version has changed as a result of major bumping marked.

[UziTech]

but the package.json says otherwise and gives the wrong impression

Good point that should be removed.

All the downstream user needs to do is be using node 8 and run npm install

Why would a downstream user be running node 8 and running npm install. The only reason I can think of someone running Node 8 is because it has been running for a while and is not going to be updated (including dependencies) any time soon.

[UziTech]

If we can't guarantee any particular version of Node works with any given marked release then it defeats the point of semver.

We have listed what our semver version mean in the versioning docs. If someone wants the latest features and security they should keep their dependencies updated. Node is a dependency of their application just like any other package from npm or any other binaries they rely on. We can't always help if our update breaks due to one of their other dependencies.
We test every change against the current LTS and latest Node versions to ensure they are working.

[alasdairhurst]

Generally a lot of the community classifies dropping support of a certain node version as a breaking (major) change to the API/package. In this case semver wasn't really an issue per say - 2.0.0 got released. The main problem is that it seems that the same problem could potentially arise in a minor or patch version in the future (i.e. making use of a node 12 feature as soon as 10 goes EOL).

Why would a downstream user be running node 8 and running npm install. The only reason I can think of someone running Node 8 is because it has been running for a while and is not going to be updated (including dependencies) any time soon.

They could be running npm install daily for the past 2 years. If everyone somehow managed to follow the same definition of semver perfectly then this user can keep the same environment and can be pulling daily features and fixes with a guarantee that everything will still work in an equivalent way after each update. Maybe their node version is long dead, maybe their dependencies have moved on and only support the latest version of node, but their module will still be pulling the older versions that are compatible with the environment that they started with.

You guys can do things you like, but it's worth at the very least documenting specifically which node versions marked supports at any given time. You mention only latest and LTS but I don't see that anywhere. Any module which depends on marked will be automatically forced to support a version of Node within that range (and this range will shrink as the product of each additional dependency), so it should be clear so that the devs of downstream modules can document accordingly for their users and so on (or decide not to upgrade). :)

[UziTech]

Generally a lot of the community classifies dropping support of a certain node version as a breaking (major) change to the API/package.

Which implies not all of the community believes that, therefor one should not just run npm install everyday believing everything should just work.

I understand your point and we have had to create fixes for minor version updates that broke supported systems.

If you would like to create a PR to update docs or package engine that would be much appreciated. 😁👍

[alasdairhurst]

https://github.com/markedjs/marked/pull/1938/files

[SMKH-PRO]

I am facing the same problem when using the library in react native, any solutions?

[UziTech]

@SMKH-PRO #2843