jsdom / webidl2js

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

Consider a better API for feeding in files #160

Open domenic opened 4 years ago

domenic commented 4 years ago

https://github.com/jsdom/webidl2js/pull/159 ran into an issue where the current API is not flexible enough to do targeted testing, because it operates on a directory level. This has always been a bit of a strange design; it's not very flexible and assumes a lot about the directory structure. I'd love to get opinions on what would be better designs.

Here are some ideas:

const transformer = new WebIDL2JS();

transformer.addSource("path/to/Attr.webidl", "path/to/Attr-impl.js");
// ... add the rest, one file at a time. The client is responsible for directory iteration.

await transformer.generate("outputDir");
const transformer = new WebIDL2JS({
  implPath(webidlFilePath) { return ...; }
  wrapperPath(webidlFilePath, implPath) { return ...; }
});

transformer.addFile("webidlFilesDir/Attr.webidl");

// Optionally add some of these as convenience methods:
transformer.addDirectory("webidlFilesDir");
transformer.addFromGlob("webidlFilesDir/**.webidl");

await transformer.generate();

Other ideas are welcome as well.

ExE-Boss commented 4 years ago

In https://github.com/jsdom/webidl2js/pull/208, I implemented addFile(…) and addDirectory(…) to match addSource(…), except with an optional impl path argument.


It also fixes the bug where:

const transformer = new WebIDL2JS();

transformer.addSource("path/to/Attr.webidl", "path/to/Attr-impl.js");
// ... add the rest, one file at a time. The client is responsible for directory iteration.

await transformer.generate("outputDir");

Would result in outputDir/Attr.js requiring path/to/Attr‑impl.js/Attr.js instead of path/to/Attr‑impl.js, because the wrapper generation code currently assumes that the impl path argument is always a directory.

domenic commented 4 years ago

I'd like us to consider and decide on the eventual shape of the whole API, and not just add things. In particular, I think addSource is a bad API with a bad name.

Can you describe how addFile and addDirectory work, including what arguments they take? And, if you disagree and think addSource should be kept, then can you include that as part of your description? Basically, before we consider working on a pull request, I'd like to see a concrete proposal, at least at the level of detail of API documentation.

domenic commented 3 years ago

Testing would be easier if we had a core API that didn't do file I/O at all, plus a wrapper API that did the file I/O. I guess that would be something like

const transformer = new WebIDL2JS();

transformer.add(idlString1, implModuleSpecifier1);
transformer.add(idlString2, implModuleSpecifier2);

for (const constructName of transformer.constructs()) {
  const result = transformer.generate(constructName);
}

const utils = transformer.utils();

Although this idea of using module specifiers for the impl is a bit fragile; it helps keep the pipeline pure string-to-string, but it makes it the caller's responsibility to get the exact path mapping right on disk. Maybe that's fine with a good wrapper API though...