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

Imported types #69

Open
TimothyGu opened this issue Sep 13, 2017 · 7 comments · May be fixed by #227
Open

Imported types #69

TimothyGu opened this issue Sep 13, 2017 · 7 comments · May be fixed by #227

Comments

@TimothyGu
Copy link
Member

The Problem

A giant project like jsdom inevitably requires many different modules to provide certain features. Some of these features are specified using Web IDL, like URL, URLSearchParams, and most recently, DOMException. @Zirro's work on css-object-model is also webidl2js-based, and a future Fetch implementation can conceivably utilize webidl2js as well, while being compatible with the greater Node.js environment.

With all of these external modules using webidl2js, type sharing can become a problem. As it currently stands, webidl2js must have knowledge of all types to generate useful type checks. For example, if URL is specified as a type for an operation argument in jsdom, it would not be validated by the generated JS file currently, because webidl2js has no idea what URL type is or how to check if a value is of that type.

A system implemented in webidl2js that makes it possible to import types from other modules would solve this problem.

The Requirements

Here, I wrote up a list of use cases such a module system should allow.

  1. The system must allow for type checks for interfaces/dictionaries/typedefs defined in other modules. In other words, the URL-type argument must be able to be automatically validated, even though the interface was defined in another module (whatwg-url in this case).

  2. Type discovery from module should be automatic after informing webidl2js of the required modules. We shouldn't have to tell webidl2js "whatwg-url includes URL and URLSearchParams interfaces"; it should figure that out automatically after we tell it to "search in whatwg-url for IDL types".

  3. The system may allow manual addition to the type inventory. There are some interfaces that are either not specified in Web IDL (e.g. ReadableStream) or notoriously difficult to implement (Window for example), that forcing it to go through webidl2js may be impractical. This isn't directly related to modules, but may be applicable in the future.

The Proposal

"webidl2js" field in package.json

This is used for autodetection of types as mentioned in point 2 above. This field shall have the following schema:

{
  "webidl2js": {
    "idl": [ string ],
    "generated": string
  }
}

The "idl" field is an array of paths pointing to available IDL fragments in the module, relative to the root of the module. This will require publishing the IDL files alongside the generated JS files in the npm bundle.

Note: this design does not necessitate that every interface be in its own .idl file, so a prepublish step to concatenate (and possibly minify) all IDL fragments can be used to reduce npm bundle size.

The "generated" field is a string pointing to the path where generated JS files for each interface/dictionary/enum may be found.

An example fragment of package.json for whatwg-url may look like the following:

{
  "webidl2js": {
    "idl": [
      "src/URL.idl",
      "src/URLSearchParams.idl"
    ],
    "generated": "lib/"
  }
}

where lib/URL.js contains the generated interface file for the URL IDL interface, and lib/URLSearchParams.js contains that for URLSearchParams.

Transformer#addModule(moduleNameOrPathToPackageJSON)

Make webidl2js transformer be aware of a new module.

if (typeof moduleNameOrPathToPackageJSON !== "string") {
  throw new TypeError("...");
}

let packageJSON;

if (moduleNameOrPathToPackageJSON.endsWith(".json")) {
  try {
    if (fs.statSync(moduleNameOrPathToPackageJSON).isFile()) {
      packageJSON = path.resolve(moduleNameOrPathToPackageJSON);
    }
  } catch (err) {
    // ignored
  }
}

if (packageJSON !== undefined) {
  // moduleNameOrPathToPackageJSON is a module name
  packageJSON = require.resolve(`${moduleNameOrPathToPackageJSON}/package.json`);
}

const pkg = require(packageJSON).webidl2js;

if (typeof pkg !== "object") {
  // ignored
  return;
} 

// During generate(), webidl2js will then read all of the IDL files specified
// in pkg.idl and mark the types defined in those files as "imported".
// When checking if a value is of an imported interface, the "is" property
// exported from the imported file will be used.

Transformer#addExternalInterface(interfaceName, pathToInterfaceFile) (optional)

This is used to satisfy requirement 3 above. interfaceName is the name of the interface, while pathToInterfaceFile is expected to be a relative path pointing to a manually created file that follows the general protocol of a webidl2js-generated interface file (i.e. implementing is() and convert() operations; exposes interface object as "interface" [and possibly "exposed"] property).

The Questions

Should the type conversion mechanism unwrap objects of an imported type? It could be argued that the objects created from a class defined in another module are implementation details of the other module, but it could also be argued that we shouldn't treat imported interfaces any differently from native ones.

If the answer to the first question is "yes", then we should consider a way to make idlUtils.implForWrapper work on objects created from any copy of webidl2js. This will probably require using Symbol.for() when declaring the webidl2js idlUtils.implSymbol to add it to the global symbol registry. However, this will make it easier to get the impl in a JSDOM-created window since the global symbol registry is shared by all Realms.

@domenic
Copy link
Member

domenic commented Sep 13, 2017

Great writeup.

I'd prefer a version that doesn't require shipping extra files. Couldn't we use the generated files exclusively? You can heuristically at least differentiate between interfaces and dictionaries, and we could just tag them directly.

Should the type conversion mechanism unwrap objects of an imported type?

I'd say yes, and indeed this ties into whether things get shared or not... I haven't thought past the point you have, there. Another alternative to the global symbol registry is some kind of shared dependency (e.g. a webidl2js-utils package) where we count on deduping, but that's fragile and not really any more protected.

@TimothyGu
Copy link
Member Author

Couldn't we use the generated files exclusively? You can heuristically at least differentiate between interfaces and dictionaries, and we could just tag them directly.

Yes, that differentiation in possible; but it wouldn't be possible to use an imported interface as a mixin, etc. if that were the case. I mean, another approach is something like a file header in the generated file like:

"use strict";

/* IDL source: interface URL { stuff; }; */

// ...

@Sebmaster
Copy link
Member

Should the type conversion mechanism unwrap objects of an imported type?

I'd say yes, and indeed this ties into whether things get shared or not...

I'm kinda against this. Don't specs usually specifically describe hooks needed by other specs? I'd think we expose these on the package as util methods, which receive a wrapper instance.

@domenic
Copy link
Member

domenic commented Sep 14, 2017

I guess I'd wait on solving the cross-package mixin use case until we have a concrete case.

@Sebmaster, I guess I see your point, but it's nice to have the always-impl rule... Otherwise you have to worry about calling public APIs on things... Again, some concrete examples would help. We've gotten lucky with DOMException so far since the constructor cannot be monkey patched and no jsdom code needs to worry about the code/message/name getters.

@TimothyGu
Copy link
Member Author

I guess I'd wait on solving the cross-package mixin use case until we have a concrete case.

Okay.

Other things we probably will not be able to use a set of heuristics to get:

  • Typedef information: typedefs don't have generated files.
  • Inheriting from an imported interface:
    // Members from this interface's inherited interfaces and their consequential interfaces.
    * inheritedMembers(seen = new Set([this.name]), root = this.name) {
    if (this.idl.inheritance && this.ctx.interfaces.has(this.idl.inheritance)) {
    if (seen.has(this.idl.inheritance)) {
    throw new Error(`${root} has an inheritance cycle`);
    }
    seen.add(this.idl.inheritance);
    yield* this.ctx.interfaces.get(this.idl.inheritance).allMembers(seen, root);
    }
    }

@qwtel
Copy link

qwtel commented Jun 11, 2018

There's another complication with regards to the URL and URLSearchParams types. They are now part of node's global context (since v10). It would be nice to alternate between a webidl2js implementation and the native implementation depending on the node version you're targeting.

I've hacked my way around webidl2js (platformparity/webidl2js@033b910) and webidl-conversions (platformparity/webidl-conversions@dd09029) to support this use case, but it's not terribly elegant.

I now have the same issue supporting ReadableStream, only this time there's no official node implementation as was the case with URL. So I guess I'll have to come up with a more generic hack to support this as well.

@domenic
Copy link
Member

domenic commented Apr 17, 2020

This is somewhat related to #81

@ExE-Boss ExE-Boss linked a pull request Jul 14, 2020 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants