Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Breaking change in 2.1.0 #2106

Closed
HunderlineK opened this issue Jun 16, 2021 · 22 comments · Fixed by #2109
Closed

Breaking change in 2.1.0 #2106

HunderlineK opened this issue Jun 16, 2021 · 22 comments · Fixed by #2109
Labels

Comments

@HunderlineK
Copy link

HunderlineK commented Jun 16, 2021

Marked version: 2.1.0

Describe the bug
The latest code uses the optional chaining operator which makes marked 2.1.0 incompatible with Node V12. It's breaking semantic release which uses Marked. Here:
5be9d6d

Thanks!

@calculuschild
Copy link
Contributor

@UziTech Shouldn't Babel be able to handle this?

@glasser
Copy link

glasser commented Jun 16, 2021

This is affecting our Node 12 CI as well.

@UziTech
Copy link
Member

UziTech commented Jun 16, 2021

const marked = require("marked") in node uses /src/marked.js not the transpiled es5 version at /lib/marked.js

The problem is that marked is only tested in lts/*(v14 currently) and latest (v16 currently) but the ?. syntax doesn't work in v12.

We had a discussion in #1938 (comment) about what versions of node marked should support

@glasser
Copy link

glasser commented Jun 16, 2021

(Dropping support for Node 12 is totally reasonable — just publish a rollback version at 2.1.1 and publish the version that doesn't support Node 12 as 3.0.0.)

@utsavkapoor
Copy link

Broke our CI too, via pulling in protobufjs - upgrading to node 14+ seems get past the issue, but we can't do that as a solution for other reasons.

@glasser
Copy link

glasser commented Jun 16, 2021

Yes, I'm discovering that the protobufjs CLI runs a bunch of unversioned (!) npm installs at runtime (!) which pull in marked recursively.

@utsavkapoor
Copy link

@glasser - Has anyone reported this to protobufjs ?

@glasser
Copy link

glasser commented Jun 16, 2021

protobufjs is barely maintained. we use our own fork of it and I'm trying to figure out a way to untangle its web of hackery, but honestly if marked@2.1.1 supported Node 12 and marked@3.0.0 was what you published as 2.1.0 it would solve this problem. "No longer runs under a major version of Node that's still in LTS" is a pretty clearly semver-major change.

@calculuschild
Copy link
Contributor

const marked = require("marked") in node uses /src/marked.js not the transpiled es5 version at /lib/marked.js

@UziTech Should Marked supply the es5 version by default? The transpiled version seems to run slightly faster on the benchmarks anyway. We already have Babel doing the work for us but our documentation doesn't give any hint on how to access it for people who need that compatibility.

@UziTech
Copy link
Member

UziTech commented Jun 16, 2021

The transpiled version seems to run slightly faster on the benchmarks anyway.

It didn't when we started transpiling it which is why we stuck with main pointing to the es6 version, but it seems to have gotten faster over time so maybe we should?

/cc @styfle

glasser added a commit to apollographql/apollo-server that referenced this issue Jun 16, 2021
Today, the `marked` project pushed a minor release that broke Node 12
compatibility: markedjs/marked#2106

`marked` is a dep of `jsdoc` which is a dep of the protobufjs CLI.
Unfortunately it turns out that the protobufjs CLI installs dependencies
including `jsdoc` *at runtime*, *unversioned*. Yikes! See `setup`
https://github.com/apollographql/protobuf.js/blob/master/cli/util.js

I tried changing our fork to not do this and move the deps to be dev
deps of this project (see `@apollo/protobufjs@2.0.0`) but that didn't
work out on the first try. So instead, let's just not require the
protobufjs CLI to work on Node 12, and also start saving time by not
constantly regenerating the code in this package. Now code is only
generated manually (`lerna run generate`) and checked in, and it can be
done on the latest Node.
glasser added a commit to apollographql/apollo-server that referenced this issue Jun 16, 2021
…ly (#5328)

Today, the `marked` project pushed a minor release that broke Node 12
compatibility: markedjs/marked#2106

`marked` is a dep of `jsdoc` which is a dep of the protobufjs CLI.
Unfortunately it turns out that the protobufjs CLI installs dependencies
including `jsdoc` *at runtime*, *unversioned*. Yikes! See `setup`
https://github.com/apollographql/protobuf.js/blob/master/cli/util.js

I tried changing our fork to not do this and move the deps to be dev
deps of this project (see `@apollo/protobufjs@2.0.0`) but that didn't
work out on the first try. So instead, let's just not require the
protobufjs CLI to work on Node 12, and also start saving time by not
constantly regenerating the code in this package. Now code is only
generated manually (`lerna run generate`) and checked in, and it can be
done on the latest Node.
@glasser
Copy link

glasser commented Jun 16, 2021

(We're working around it by changing our project's build system to not run the protobufjs CLI automatically on every install but just manually with checking in the generated code, which means the CLI doesn't have to run on every Node version we support.)

@UziTech UziTech mentioned this issue Jun 16, 2021
5 tasks
@brendanmc6
Copy link

brendanmc6 commented Jun 16, 2021

For those running into this problem with Next.js on Vercel (and I guess any other hosted build platform)

22:00:37.460 | Error occurred prerendering page "/404". Read more: https://nextjs.org/docs/messages/prerender-error
22:00:37.460 | /vercel/path0/node_modules/marked/src/marked.js:158
22:00:37.460 | const prevRenderer = extensions.renderers?.[ext.name];

You can fix this by upgrading the node version to 14x in your Vercel project settings!

@david-amores-anz
Copy link

david-amores-anz commented Jun 16, 2021

Just FYI, this is also breaking Twilio CLI's flex plugin:

$ DEBUG=* twilio flex:plugins:deploy --patch --changelog="test deploy"

/Users/xxxxxx/.twilio-cli/node_modules/marked/src/marked.js:158
          const prevRenderer = extensions.renderers?.[ext.name];
                                                    ^

SyntaxError: Unexpected token '.'
    at compileFunction (<anonymous>)
    at wrapSafe (internal/modules/cjs/loader.js:1053:16)
    at Module._compile (internal/modules/cjs/loader.js:1101:27)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:1157:10)
    at Module.load (internal/modules/cjs/loader.js:985:32)
    at Function.Module._load (internal/modules/cjs/loader.js:878:14)
    at Module.require (internal/modules/cjs/loader.js:1025:19)
    at require (internal/modules/cjs/helpers.js:72:18)
    at Object.<anonymous> (/Users/xxxxxx/.twilio-cli/node_modules/flex-dev-utils/dist/marked.js:7:32)
    at Module._compile (internal/modules/cjs/loader.js:1137:30)

Thanks for looking into it!

khawkins98 added a commit to visual-framework/vf-core that referenced this issue Jun 16, 2021
* Pin `marked` dependency to 2.0.7, as 2.1.0 breaks support in node 12.
* Background: markedjs/marked#2106
nitinja pushed a commit to visual-framework/vf-core that referenced this issue Jun 16, 2021
* Featuer: pin marked

* Pin `marked` dependency to 2.0.7, as 2.1.0 breaks support in node 12.
* Background: markedjs/marked#2106

* Add PR link
@taikulawo
Copy link

tldr have broken on node 12 https://tldr.sh/ 😭

@cctv1237
Copy link

protobufjs is using markedjs and looks like did not lock the version, so it's blocked our production release. Save our live please 😭

installing semver@^7.1.2
installing chalk@^4.0.0
installing jsdoc@^3.6.3
installing uglify-js@^3.7.7
installing espree@^7.0.0
installing estraverse@^5.1.0
/temp/node_modules/protobufjs/cli/node_modules/marked/src/marked.js:158
          const prevRenderer = extensions.renderers?.[ext.name];
                                                    ^

SyntaxError: Unexpected token '.'
    at new Script (vm.js:88:7)
    at createScript (vm.js:263:10)
    at Object.runInThisContext (vm.js:311:10)
    at wrapSafe (internal/modules/cjs/loader.js:1034:15)
    at Module._compile (internal/modules/cjs/loader.js:1097:27)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:1153:10)
    at Module.load (internal/modules/cjs/loader.js:977:32)
    at Object.load (/temp/node_modules/protobufjs/cli/node_modules/requizzle/lib/loader.js:105:18)
    at Requizzle.requizzle (/temp/node_modules/protobufjs/cli/node_modules/requizzle/lib/requizzle.js:87:31)
    at infectProxy (/temp/node_modules/protobufjs/cli/node_modules/requizzle/lib/loader.js:79:31)
    at Module.targetModule.require (/temp/node_modules/protobufjs/cli/node_modules/requizzle/lib/loader.js:97:44)
    at require (internal/modules/cjs/helpers.js:77:18)
    at Object.<anonymous> (/temp/node_modules/protobufjs/cli/node_modules/jsdoc/lib/jsdoc/util/markdown.js:8:16)
    at Module._compile (internal/modules/cjs/loader.js:1133:30)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:1153:10)
    at Module.load (internal/modules/cjs/loader.js:977:32)
/temp/node_modules/protobufjs/cli/pbts.js:133
                throw err;
                ^

Error: code 1
    at ChildProcess.<anonymous> (/temp/node_modules/protobufjs/cli/pbts.js:130:27)
    at ChildProcess.emit (events.js:322:22)
    at maybeClose (internal/child_process.js:1021:16)
    at Process.ChildProcess._handle.onexit (internal/child_process.js:286:5)
error Command failed with exit code 1.
child_process.js:669
    throw err;
    ^

@4eb0da
Copy link

4eb0da commented Jun 16, 2021

I've found some workaround, maybe it will help someone

Call it before using protobufjs / pbjs

cd node_modules/protobufjs/cli && npm i && npm i marked@2.0

@josefschabasser
Copy link

Hi! This issue just bit us.

I've found some workaround, maybe it will help someone

Call it before using protobufjs / pbjs

cd node_modules/protobufjs/cli && npm i && npm i marked@2.0

Is this really the only solution right now? We're using yarn v2 and tried the resolutions field in our package.json but it doesn't work. yarn why marked yields no output at all.

@Nilos
Copy link

Nilos commented Jun 16, 2021

We're using yarn too and fixed this in the following way:

yarn add --dev marked@2.0.3
yarn add --dev jsdoc@^3.6.3

Alternatively you can also force marked@^2.0.3 to use 2.0.3.

The main thing is to install jsdoc yourself.

@HunderlineK
Copy link
Author

As a workaround for our use case, we fixed the semantic-release to "semantic-release": "17.3.6",

@github-actions
Copy link

🎉 This issue has been resolved in version 2.1.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

@taycaldwell
Copy link

🎉 This issue has been resolved in version 2.1.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

This issue still exists in version 2.1.1.

Revert and release breaking change in a new major version please.

@josefschabasser
Copy link

tada This issue has been resolved in version 2.1.1 tada
The release is available on:

Your semantic-release bot packagerocket

This issue still exists in version 2.1.1.

Revert and release breaking change in a new major version please.

For us (local and CI) it's resolved. Maybe you should delete your node_modules and reinstall?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.