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

Installing unpinned vega-lite package breaks Node 12 support #40

Closed
0x2b3bfa0 opened this issue Sep 7, 2021 · 4 comments · Fixed by #41 or iterative/cml#720
Closed

Installing unpinned vega-lite package breaks Node 12 support #40

0x2b3bfa0 opened this issue Sep 7, 2021 · 4 comments · Fixed by #41 or iterative/cml#720
Assignees
Labels
p0-critical bugs & errors

Comments

@0x2b3bfa0
Copy link
Member

0x2b3bfa0 commented Sep 7, 2021

We're installing all the vega packages without any version pinning, and with methods other than package-lock.json 🙀

setup-cml/src/utils.js

Lines 35 to 37 in fbe9608

`${sudo} npm install -g canvas vega vega-cli vega-lite @dvcorg/cml${
version !== 'latest' ? `@${version}` : ''
}`

Tests were failing for #38* because we depend on vega-lite and it's trying to use a nullish coalescing operator ?? on Node JS 12, but wasn't supported until 14.

See also vega/vega-lite#v5.0.0 release notes: no more ES5 builds.


* Tests were failing also for the master branch; we know it thanks to the scheduled tests introduced with #37.

@0x2b3bfa0 0x2b3bfa0 added the p0-critical bugs & errors label Sep 7, 2021
@0x2b3bfa0 0x2b3bfa0 self-assigned this Sep 7, 2021
@0x2b3bfa0
Copy link
Member Author

Pinning behavior of actions/setup-node

The node-version input is optional. If not supplied, the node version from PATH will be used. However, it is recommended to always specify Node.js version and don't rely on the system one.

For information regarding locally cached versions of Node.js on GitHub hosted runners, check out GitHub Actions Virtual Environments.

@0x2b3bfa0
Copy link
Member Author

Default Node JS version on GitHub Actions

GitHub Actions Virtual Environments are in Node 14 since 2020-11-02, as per actions/runner-images#1953 (comment), so supporting 12 is not neccessary anymore, but we're going to keep it around for compatibility.

@0x2b3bfa0
Copy link
Member Author

Should we install vega and related packages?

@DavidGOrtega
Copy link
Contributor

We decided to include vega as far as I remember, it was removed for a while a then we came back

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
p0-critical bugs & errors
Projects
None yet
2 participants