Revert "perf: reuse TextDecoder instance (#2863)"

[panva]

This reverts commit 2091dcf.

See

• deps: update undici to 6.10.2 node#52175 (comment)
• perf: reuse TextDecoder instance #2863 (comment)

[codecov-commenter]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 93.50%. Comparing base (ef25209) to head (230b784).
Report is 1 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2999      +/-   ##
==========================================
- Coverage   93.50%   93.50%   -0.01%     
==========================================
  Files          89       89              
  Lines       24223    24220       -3     
==========================================
- Hits        22649    22646       -3     
  Misses       1574     1574              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[ronag]

ACtually doesn't this just move the issue to when websocket is used? Should we maybe not pass fatal? Does the spec require that? If it does, then maybe we should disable websocket for builds without ICU?

[mcollina]

lgtm

[panva]

ACtually doesn't this just move the issue to when websocket is used? Should we maybe not pass fatal? Does the spec require that? If it does, then maybe we should disable websocket for builds without ICU?

I don't particularly care how this moves forward but the revert is needed to unblock a Node.js version bump.

[Uzlopak]

It actually revealed that there is an issue with no-icu cases.

[panva]

It actually revealed that there is an issue with no-icu cases.

Which, now that we know about it, we can take all the time in the world to contemplate a fix for. In the meantime, node's bundled fetch is lagging behind on fixes which is why this revert is proposed.

[Uzlopak]

I would atleast expect that you open an issue here, so that it keeps tracked.

[Uzlopak]

Approving but somebody should create an issue.

[panva]

I would atleast expect that you open an issue here, so that it keeps tracked.

Noted.

#3002

[panva]

@mcollina can you do a release? Node nightly is currently lagging on 6.6.2

[ronag]

6.10.2 published

[KhafraDev]

Does the spec require that?

yes, we're meant to close the connection if we receive invalid utf-8

If it does, then maybe we should disable websocket for builds without ICU?

It actually revealed that there is an issue with no-icu cases.

It's already essentially disabled:

• Open a WebSocket connection (on a build without icu)
• The connection closes if it receives a message.
• The close frame has no reason if we receive a close frame.

We might be able to support builds without icu by using Buffer.isUTF8, but it should only be used when we can't use TextDecoder for perf. I'm not really sure if it's available in builds without ICU and I don't have the time to test myself.