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

Making util.TextDecoder and util.TextEncoder available on the global object #20365

Closed
MarkTiedemann opened this issue Apr 27, 2018 · 6 comments
Closed

Comments

@MarkTiedemann
Copy link
Contributor

@MarkTiedemann MarkTiedemann commented Apr 27, 2018

  • Version: v10.0.0
  • Platform: Microsoft Windows [Version 10.0.16299.371]
  • Subsystem: util

Just wondering since the WHATWG URL API was made available on the global object in Node v10.0.0: Should the WHATWG TextEncoder and TextDecoder APIs (that are currectly available via require('util')) be made available on the global object, too, or are there any reasons for not doing so?

PS: If this should be done, I'd love to start working on a PR for it.

@addaleax
Copy link
Member

@addaleax addaleax commented Apr 27, 2018

Loading

@mcollina
Copy link
Member

@mcollina mcollina commented Apr 27, 2018

+1 as semver-major.

Loading

@jasnell
Copy link
Member

@jasnell jasnell commented Apr 27, 2018

+1 as a major

Loading

@BridgeAR
Copy link
Member

@BridgeAR BridgeAR commented May 10, 2018

Just as a note: the more we add as globals, the harder it gets to lazy load parts of core to improve startup time and to minimize memory overhead.

Loading

@MarkTiedemann
Copy link
Contributor Author

@MarkTiedemann MarkTiedemann commented May 10, 2018

@BridgeAR I agree with you in general. I do think, however, in this particular case const { deprecate } = NativeModule.require('internal/util'); is called before const { TextEncoder, TextDecoder } = NativeModule.require('internal/util');. Not sure how NativeModule.require works internally, but I would expect the second call not to add large overhead. Is that correct?

EDIT: Correction: deprecate is required after TextEncoder and TextDecoder.

Loading

@BridgeAR
Copy link
Member

@BridgeAR BridgeAR commented May 10, 2018

See #20567. It is the combination of all those files at startup time. It is not about this specifically (it would be fine as it is right now) but I just want to point out in general that every added global is making it more difficult to load less things on startup. Using getters for the globals would help but at least for console it should not be a getter due to the spec. I do not know if that is also here the case but I guess it is.

Loading

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
5 participants