jsdom / webidl2js

Auto-generate JS class structures for Web IDL specifications
MIT License
79 stars 30 forks source link

Imported types #69

Open TimothyGu opened 7 years ago

TimothyGu commented 7 years ago

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 commented 7 years ago

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 commented 7 years ago

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 commented 7 years ago

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 commented 7 years ago

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 commented 7 years ago

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:

qwtel commented 6 years ago

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 commented 4 years ago

This is somewhat related to https://github.com/jsdom/webidl2js/issues/81