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

Prepare for Node 18 upgrade #1387

Merged
merged 2 commits into from
Jun 15, 2022
Merged

Prepare for Node 18 upgrade #1387

merged 2 commits into from
Jun 15, 2022

Conversation

elenatanasoiu
Copy link
Contributor

@elenatanasoiu elenatanasoiu commented Jun 15, 2022

We'd like to pave the way to upgrade from Node 16.13.0 (current version) to Node 18 (when VSCode decides to switch to it).

As we discovered in #1378, our build is currently breaking on Node 18 for two reasons:

  • source-map@0.7.3 does not work on Node 18
  • our current version of webpack relies on OpenSSL for its hashing algorithm but OpenSSL is not supported by default from Node 17 onwards. While we can provide a legacy flag to revert to using OpenSSL in Node 18, this flag doesn't work with our current version of Node (16.13.0) and is actually re-activating insecure behaviour.

To address these issues we're:

  • upgrading to source-map@0.7.4 which has very recently been released and works with Node 18
  • switch to a newer version of webpack which doesn't rely on OpenSSL

Happily, these packages run with both Node 16.13.0 and Node 18.

Checklist

  • CHANGELOG.md has been updated to incorporate all user visible changes made by this pull request.
  • Issues have been created for any UI or other user-facing changes made by this pull request.
  • [Maintainers only] If this pull request makes user-facing changes that require documentation changes, open a corresponding docs pull request in the github/codeql repo and add the ready-for-doc-review label there.

@elenatanasoiu elenatanasoiu changed the title Attempt to fix broken build in node 18. Attempt to fix broken build in node 18 Jun 15, 2022
@aeisenberg
Copy link
Contributor

If this works, it is much better than my approach. Also need to make sure the build still runs using node 16 (or at least the local build works).

A new release of source-map was pushed 10 days ago:
https://github.com/mozilla/source-map/releases/tag/v0.7.4

It contains a fix for building on Node 18 (which was added in Oct
2020): mozilla/source-map#423.

Let's make use of it!
We're upgrading the minimum version of webpack from 5.28.0 to 5.62.2
since this version doesn't rely on OpenSSL for its hashing algorithm so
it wouldn't need legacy OpenSSL support when we decide to upgrade to
Node 18.

This allows us to build our extension on Node 18:
https://github.com/github/vscode-codeql/runs/6904100934?check_suite_focus=true

Happily, this also works fine with our current version of Node (16.13.0).
@elenatanasoiu elenatanasoiu changed the title Attempt to fix broken build in node 18 Prepare for Node 18 upgrade Jun 15, 2022
@elenatanasoiu elenatanasoiu marked this pull request as ready for review June 15, 2022 17:11
@elenatanasoiu elenatanasoiu requested a review from a team as a code owner June 15, 2022 17:11
@elenatanasoiu elenatanasoiu requested a review from a team June 15, 2022 17:22
Copy link
Contributor

@aeisenberg aeisenberg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice. Next step is to run our CI on both node 16 and node 18. That can come later.

@elenatanasoiu elenatanasoiu merged commit c63f0c0 into main Jun 15, 2022
@elenatanasoiu elenatanasoiu deleted the elenatanasoiu/node-18 branch June 15, 2022 18:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants