Open MicahZoltu opened 5 years ago
I also implement TextEncoder
and TextDecoder
in miniSphere so :+1: to not forcing a dependency on lib.dom.d.ts
just to use the Encoding API.
It is worth noting that console.log
is duplicated between dom
and node
definition files, which means even though you can safely use console.log
in both browser and node, you must depend on either dom
or @types/node
in order to actually use it, which can result in incorrectly depending on something from whichever library you include and the compiler not correctly yelling at you.
@rbuckton suggests that a new file lib.dom.textencoder.d.ts could contain the TextEncoder types, and then the node types could reference that using a /// <lib=.../>
reference. He pointed out that the URL API also comes from a WHATWG spec and has the same problem.
We would want to make this change all at once, because each Typescript version that introduces a new lib.dom.*.d.ts to @types/node
would require a typesVersion fork in @types/node
so that the old versions don't reference a file that doesn't exist. In other words, at least the types for node 11 and node 12 would have to bifurcate to support Typescript versions before-this-change vs after-this-change.
On the other hand, the situation now is bad because the node type authors can't include TextEncoder or URL because it would cause conflicts for projects that do want to have both node and DOM types. So individual projects have to incorrectly add DOM types.
Node type authors, can you provide an opinion on how much work a typesVersion fork for node 11 and 12 would add to updating node types? What do you think of the benefit?
@jkomyno, @a-tarasyuk, @alvis, @r3nya, @btoueg, @brunoscheufler, @smac89, @tellnes, @touffy, @DeividasBakanas, @eyqs, @Flarna, @Hannes-Magnusson-CK, @KSXGitHub, @hoo29, @kjin, @ajafff, @islishude, @mwiktorczyk, @matthieusieben, @mohsen1, @n-e, @octo-sniffle, @parambirs, @eps1lon, @SimonSchick, @ThomasdenH, @WilcoBakker, @wwwy3y3, @ZaneHannanAU, @jeremiergz, @samuela, @kuehlein, @j-oliveras, @bhongy,
Just to clarify, is the goal here to no longer require dom typings to be loaded when we load node typings?
Should something similar be done to console
?
Yes, for console, TextEncoder, and URL (and their associated types). Making users include dom typings is inaccurate if they only want those types.
I'm all for splitting the shared components out of node typings we've had lots of trouble with those anyways..., what is the concern with breaking changes here, ideally the node typings would just add lib references and we should be good.
@SimonSchick Old typescript versions won't have the new files for the shared components, so we'll need to maintain two versions of node side-by-side via typesVersions
directories. It would apply to all node versions since the shared components were made global, so node 10+ I think.
While TextEncoder was moved in NodeJS 11 to match the web versions API, console has matched for a long time (not just NodeJS 11+). In a perfect world, all versions of node types would get console
global from some shared location, while only node 11+ types would get TextEncoder from a shared location. I recognize this may complicate things even more though.
Search Terms
TextEncoder
Suggestion
Put
(window|global).TextEncoder
in a library that is automatically available to both browser and nodejs.Use Cases
TextEncoder
now exists onwindow
in all major browsers as well as onglobal
in NodeJS (as of NodeJS 11). At the moment, theTextEncoder
definition lives inlib.dom.d.ts
which means it isn't available when working on a project/library that is targeting both NodeJS and Browser.I don't mind submitting a PR, but I don't know where the appropriate location for such a change is, and how to reconcile it properly with removal from
lib.dom.d.ts
. It isn't part of an annual ES specification, but it should go somewhere that is automatically included in both browser and node now that it is implemented by both. The specification for it can be found at https://encoding.spec.whatwg.org/ and I believe that is what has been implemented in NodeJS.Checklist
My suggestion meets these guidelines: