fix: debugger failing on Node <=12 #1627
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
TIL that assigning an invalid value to `url.hostname` silently fails if invalid. Fixes microsoft/vscode#167353
Fixes #1624 Updates @vscode/l10n to allow it to be tree-shaken away. Also moves `checkContentHash` that likewise had a dependency on more modern language features. I think webpack was just extra aggressive about tree shaking before and assumed `new Hasher()` was side effect free. Which is was, it's just a big assumption to have made. And when I had updated versions in our pipelines, I accidentally updated the minspec to no longer target Node 8 🤦♂️. Targeting the latest 10 should be good. Node 8 thankfully <0.25% of users nowadays.
lramos15
previously approved these changes
Mar 31, 2023
connor4312
force-pushed
the
connor4312/issue1624
branch
from
04120ef
to
a42b19f
Compare
|
I undid the "minspec" changes for this patch, since I realize our test and build infra no longer works on Node <14 so we can't just use that version for the whole pipeline. Will need to figure a way to use Node 10 only during tests. |
rebornix
approved these changes
Mar 31, 2023
connor4312
added a commit
that referenced
this pull request
Mar 31, 2023
* fix: sourcemap lookups on ipv6 localhost addresses TIL that assigning an invalid value to `url.hostname` silently fails if invalid. Fixes microsoft/vscode#167353 * fix: debugger failing on Node <=12 Fixes #1624 Updates @vscode/l10n to allow it to be tree-shaken away. Also moves `checkContentHash` that likewise had a dependency on more modern language features. I think webpack was just extra aggressive about tree shaking before and assumed `new Hasher()` was side effect free. Which is was, it's just a big assumption to have made. And when I had updated versions in our pipelines, I accidentally updated the minspec to no longer target Node 8 🤦♂️. Targeting the latest 10 should be good. Node 8 thankfully <0.25% of users nowadays. * keep broken minspec for the moment
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #1624
Updates @vscode/l10n to allow it to be tree-shaken away.
Also moves
checkContentHashthat likewise had a dependency on moremodern language features. I think webpack was just extra aggressive
about tree shaking before and assumed
new Hasher()was side effectfree. Which is was, it's just a big assumption to have made.
And when I had updated versions in our pipelines, I accidentally updated
the minspec to no longer target Node 8 🤦♂️. Targeting the latest 10 should
be good. Node 8 thankfully <0.25% of users nowadays.