-
Notifications
You must be signed in to change notification settings - Fork 9k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
fix(jsdom): cumulative Jest updates with support for TextEncoder/Text…
…Decoder (#8111)
- Loading branch information
Showing
4 changed files
with
6,608 additions
and
1,725 deletions.
There are no files selected for viewing
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
Oops, something went wrong.
b89e885
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @tim-lai,
Why using
util
? What is it's purpose here?As jest runs in Node.js,
TextEncoder
andTextDecoder
are part of global object in our lowest Node.js we claim to support - 12.4.x. Can you please elaborate about this change?In order to fix the regression described in #8136, we have to uninstall the
util
npm package and use util directly from node. Modern versions of Node.js allow to use explicit schema to signify where the code comes from:...but given our minimal supported version of Node.js is currently is 12.4.x which doesn't support explicit
node:
schema, we have to be careful and not installutil
npm package.b89e885
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@char0n
util
was added to makeTextEncoder
andTextDecoder
work in Jest. If using an explicit schemanode:util
works, that's great.My intention was to only support >Node 14.16, as Node 12.x is EOL and no longer in maintenance. I meant to update this in our docs. Also fyi, Jenkins jobs were recently updated to Node 14.19 as part of this change.
b89e885
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
util
vendor library is not needed, that's what I'm trying to say.TextEncoder
andTextDecoder
are native part of node.js versions we're using.It will not work on Node.js 12.x, that's why I didn't use it now and we must make sure we don't install unneded
util
npm package.I've issued PR that should remedy this for future - #8142
One thing that we should add there explicitly is the information about what is minimal version of Node.js (from
Maintenance LTS
) that we support.