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

lib: expose DOMException as global #39176

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -349,6 +349,7 @@ module.exports = {
BigInt: 'readable',
BigInt64Array: 'readable',
BigUint64Array: 'readable',
DOMException: 'readable',
Event: 'readable',
EventTarget: 'readable',
MessageChannel: 'readable',
Expand Down
10 changes: 10 additions & 0 deletions doc/api/globals.md
Original file line number Diff line number Diff line change
Expand Up @@ -380,6 +380,15 @@ added: v0.0.1

[`setTimeout`][] is described in the [timers][] section.

## `DOMException`
<!-- YAML
added: REPLACEME
-->

<!-- type=global -->

The WHATWG `DOMException` class. See [`DOMException`][] for more details.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe Web IDL instead of WHATWG ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://dom.spec.whatwg.org/

hmmm, isn't it WHATWG DOMException?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

DOMException is defined here: https://heycam.github.io/webidl/#idl-DOMException (the DOM spec links to that too)
WHATWG is the group that writes the standards and it feels weird to me to use that name as the reference. Kind of like if we said "The TC39 SyntaxError".

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What confused me is that likely URL has an IDL way:

[Exposed=(Window,Worker),
 LegacyWindowAlias=webkitURL]
interface URL {
  constructor(USVString url, optional USVString base);

  stringifier attribute USVString href;
  readonly attribute USVString origin;
           attribute USVString protocol;
           attribute USVString username;
           attribute USVString password;
           attribute USVString host;
           attribute USVString hostname;
           attribute USVString port;
           attribute USVString pathname;
           attribute USVString search;
  [SameObject] readonly attribute URLSearchParams searchParams;
           attribute USVString hash;

  USVString toJSON();
};

[Exposed=(Window,Worker)]
interface URLSearchParams {
  constructor(optional (sequence<sequence<USVString>> or record<USVString, USVString> or USVString) init = "");

  undefined append(USVString name, USVString value);
  undefined delete(USVString name);
  USVString? get(USVString name);
  sequence<USVString> getAll(USVString name);
  boolean has(USVString name);
  undefined set(USVString name, USVString value);

  undefined sort();

  iterable<USVString, USVString>;
  stringifier;
};

So when should we use Web IDL and when should we use WHATWG?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should stop using WHATWG completely and always use Web instead. I know that right now the situation is mixed. In the documentation we have "WHATWG URL" and "WHATWG Encoding", but on the other hand we have "Web Streams" and "Web Crypto"...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You mean use:

Web URL / Web TextEncoder?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would just remove the The WHATWG DOMException class. sentence entirely.


## `TextDecoder`
<!-- YAML
added: v11.0.0
Expand Down Expand Up @@ -430,6 +439,7 @@ The object that acts as the namespace for all W3C
[Mozilla Developer Network][webassembly-mdn] for usage and compatibility.

[`AbortController`]: https://developer.mozilla.org/en-US/docs/Web/API/AbortController
[`DOMException`]: https://developer.mozilla.org/en-US/docs/Web/API/DOMException
[`EventTarget` and `Event` API]: events.md#event-target-and-event-api
[`MessageChannel`]: worker_threads.md#worker_threads_class_messagechannel
[`MessageEvent`]: https://developer.mozilla.org/en-US/docs/Web/API/MessageEvent/MessageEvent
Expand Down
2 changes: 2 additions & 0 deletions lib/.eslintrc.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,8 @@ rules:
message: "Use `const { Atomics } = globalThis;` instead of the global."
- name: Buffer
message: "Use `const { Buffer } = require('buffer');` instead of the global."
- name: DOMException
message: "Use lazy function `const { lazyDOMException } = require('internal/util');` instead of the global."
- name: Event
message: "Use `const { Event } = require('internal/event_target');` instead of the global."
- name: EventTarget
Expand Down
17 changes: 16 additions & 1 deletion lib/internal/bootstrap/node.js
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ const {
globalThis,
} = primordials;
const config = internalBinding('config');
const { deprecate } = require('internal/util');
const { deprecate, lazyDOMExceptionClass } = require('internal/util');

setupProcessObject();

Expand Down Expand Up @@ -201,6 +201,12 @@ if (!config.noBrowserGlobals) {
exposeInterface(globalThis, 'URL', URL);
// https://url.spec.whatwg.org/#urlsearchparams
exposeInterface(globalThis, 'URLSearchParams', URLSearchParams);
exposeGetterAndSetter(globalThis,
'DOMException',
lazyDOMExceptionClass,
(value) => {
exposeInterface(globalThis, 'DOMException', value);
});

const {
TextEncoder, TextDecoder
Expand Down Expand Up @@ -484,6 +490,15 @@ function exposeInterface(target, name, interfaceObject) {
});
}

function exposeGetterAndSetter(target, name, getter, setter = undefined) {
ObjectDefineProperty(target, name, {
enumerable: false,
configurable: true,
get: getter,
set: setter,
});
}

// https://heycam.github.io/webidl/#define-the-operations
function defineOperation(target, name, method) {
ObjectDefineProperty(target, name, {
Expand Down
13 changes: 9 additions & 4 deletions lib/internal/util.js
Original file line number Diff line number Diff line change
Expand Up @@ -442,11 +442,15 @@ function createDeferredPromise() {
return { promise, resolve, reject };
}

let DOMException;
let _DOMException;
const lazyDOMExceptionClass = () => {
_DOMException ??= internalBinding('messaging').DOMException;
return _DOMException;
};

const lazyDOMException = hideStackFrames((message, name) => {
if (DOMException === undefined)
DOMException = internalBinding('messaging').DOMException;
return new DOMException(message, name);
_DOMException ??= internalBinding('messaging').DOMException;
return new _DOMException(message, name);
});

module.exports = {
Expand All @@ -466,6 +470,7 @@ module.exports = {
isInsideNodeModules,
join,
lazyDOMException,
lazyDOMExceptionClass,
normalizeEncoding,
once,
promisify,
Expand Down
11 changes: 11 additions & 0 deletions test/parallel/test-global-domexception.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
'use strict';

require('../common');

const assert = require('assert');

assert.strictEqual(typeof DOMException, 'function');

assert.throws(() => {
atob('我要抛错!');
}, DOMException);
2 changes: 0 additions & 2 deletions test/wpt/test-atob.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,6 @@ runner.setFlags(['--expose-internals']);
runner.setInitScript(`
const { internalBinding } = require('internal/test/binding');
const { atob, btoa } = require('buffer');
const { DOMException } = internalBinding('messaging');
global.DOMException = DOMException;
`);

runner.runJsTests();
10 changes: 0 additions & 10 deletions test/wpt/test-url.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,4 @@ const { WPTRunner } = require('../common/wpt');

const runner = new WPTRunner('url');

// Needed to access to DOMException.
runner.setFlags(['--expose-internals']);

// DOMException is needed by urlsearchparams-constructor.any.js
runner.setInitScript(`
const { internalBinding } = require('internal/test/binding');
const { DOMException } = internalBinding('messaging');
global.DOMException = DOMException;
`);

runner.runJsTests();