Add a README documenting everything #51

domenic · 2017-08-19T04:25:53Z

Closes #1.

Also review from @Zirro would be cool; not sure why they're not part of the org yet. Will try to fix that.

Closes #1.

TimothyGu

Thanks for doing this!

TimothyGu · 2017-08-19T04:48:21Z

README.md

+const implModule = require("../implementations/SomeInterface-impl.js");
+
+function SomeInterface() {
+  throw TypeError("Illegal constructor");


Missing new.

TimothyGu · 2017-08-19T04:48:57Z

README.md

+  }
+
+  const args = [];
+  args[0] = conversions["DOMString"](args[0], {


Missing arguments copying. Alternatively arguments[0] can be used instead.

TimothyGu · 2017-08-19T04:49:26Z

README.md

+  obj[impl] = new implModule.implementation(constructorArgs, privateData);
+  return obj;
+}
+```


Export interface = SomeInterface too?

TimothyGu · 2017-08-19T04:51:45Z

README.md

+Notable missing features include:
+
+- Legacy platform objects, i.e. those using `getter`/`setter`/`deleter` declarations ([in progress](https://github.com/jsdom/webidl2js/pull/48))
+- Partial interfaces


We do support partial interfaces:

webidl2js/lib/transformer.js

Lines 128 to 140 in 90c96b2

case "interface":

if (!instruction.partial) {

break;

}

if (this.options.suppressErrors && !interfaces.has(instruction.name)) {

break;

}

oldMembers = interfaces.get(instruction.name).idl.members;

oldMembers.push(...instruction.members);

extAttrs = interfaces.get(instruction.name).idl.extAttrs;

extAttrs.push(...instruction.extAttrs);

break;

Ah, I missed the ! on that line

TimothyGu · 2017-08-19T04:52:01Z

README.md

+  - Class strings (with the semantics of [heycam/webidl#357](https://github.com/heycam/webidl/pull/357))
+- Dictionary types
+- Union types
+- Callback function types


Not entirely IIRC.

TimothyGu · 2017-08-19T04:52:52Z

README.md

+* webidl2js attempts to generate informative error messages using what it knows.
+
+For more examples, you can check out the `test/` directory (with the generated output being in `test/__snapshots__`). Alternately, you can install [jsdom](https://www.npmjs.com/package/jsdom), [whatwg-url](https://www.npmjs.com/package/whatwg-url), or [domexception](https://www.npmjs.com/package/domexception) from npm and check out their source code. (Note that browsing them on GitHub will not work, as we do not check the generated files into Git, but instead generate them as part of publishing the package to npm.)
+


Would be good to have an example implementation file.

- Also: add exports.is - Also: use consistent - for bullets

TimothyGu · 2017-08-19T06:08:34Z

README.md

+- `[LenientSetter]`
+- `[LenientThis]`
+- `[NamedConstructor]`
+- `[OverrideBuiltins]`


In progress too.

TimothyGu · 2017-08-19T06:10:54Z

README.md

+
+### `[Reflect]`
+
+The `[Reflect]` extended attribute is implemented to call `this.getAttribute` or `this.setAttribute` and process the input our output using [webidl-html-reflector](https://github.com/domenic/webidl-html-reflector), generating both a getter and a setter. If `[Reflect]` is specified, the implementation class does not need to implement any getter or setter logic.


What's webidl-html-reflector? grepping it in both jsdom and webidl2js returns nothing. We have https://github.com/jsdom/webidl2js/blob/master/lib/reflector.js BTW.

Ah, I forgot that this switched when @Sebmaster created webidl2js instead of my old https://github.com/domenic/webidl-class-generator. (Which I should probably tombstone...)

Zirro · 2017-08-19T11:03:33Z

README.md

+
+The main module's default export is a class which you can construct with a few options:
+
+- `implSuffix`: a suffix used, if any, to find files within the soruce directory based on the IDL file name.


soruce directory

Zirro · 2017-08-19T11:26:10Z

Nice work! This will surely make both webidl2js and jsdom easier to approach for new contributors.

Perhaps a brief section explaining the most common methods used to work with the generated code - create(), createImpl(), more? - could be helpful as well.

(Thanks for the invitation!)

Sebmaster · 2017-08-19T15:19:11Z

We should document the wrapper vs impl instance difference, and how it can bite you if you do something weird with your args? I think we fixed arrays yeah? But there might be other cases where that can become a problem.

domenic · 2017-08-19T17:15:50Z

OK, I want to start a new PR documenting the generated wrapper files API, as I think it will require more iteration (e.g. I'll have to educate myself on how the exported API looks like for typedefs and unions and such). We can also cover the wrapper vs. impl difference there.

In the meantime I've updated according to the great review comments. Happy for people to do another pass.

TimothyGu · 2017-08-19T23:52:49Z

I'll have to educate myself on how the exported API looks like for typedefs and unions and such

Typedefs are internally converted to its definition (i.e. exactly what you would expect). Nothing too special there.

Unions are treated with fully spec-complaint behavior (including nested unions, error checking against indistinguishable member types, and extended attributes on types).

For both you can check TypedefsAndUnions.idl and its snapshot.

Add a README documenting everything

5e8f4d6

Closes #1.

domenic requested review from Sebmaster and TimothyGu August 19, 2017 04:25

TimothyGu reviewed Aug 19, 2017

View reviewed changes

Address code review comments

8b4155d

- Also: add exports.is - Also: use consistent - for bullets

TimothyGu reviewed Aug 19, 2017

View reviewed changes

Zirro reviewed Aug 19, 2017

View reviewed changes

domenic mentioned this pull request Aug 19, 2017

Argument name "obj" in generated wrapper file exports seems wrong #54

Closed

Address more code review comments

42c6af3

This was referenced Aug 19, 2017

Add docs on the wrapper API and writing impls #60

Merged

Implement legacy platform object + others #48

Merged

TimothyGu approved these changes Aug 19, 2017

View reviewed changes

domenic merged commit 0e4eb1d into master Aug 20, 2017

domenic deleted the readme branch August 20, 2017 04:58

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add a README documenting everything #51

Add a README documenting everything #51

domenic commented Aug 19, 2017

TimothyGu left a comment

TimothyGu Aug 19, 2017

TimothyGu Aug 19, 2017

TimothyGu Aug 19, 2017

TimothyGu Aug 19, 2017

domenic Aug 19, 2017

TimothyGu Aug 19, 2017

TimothyGu Aug 19, 2017

TimothyGu Aug 19, 2017

TimothyGu Aug 19, 2017

domenic Aug 19, 2017

Zirro Aug 19, 2017

Zirro commented Aug 19, 2017 •

edited

Loading

Sebmaster commented Aug 19, 2017

domenic commented Aug 19, 2017

TimothyGu commented Aug 19, 2017

	case "interface":
	if (!instruction.partial) {
	break;
	}

	if (this.options.suppressErrors && !interfaces.has(instruction.name)) {
	break;
	}
	oldMembers = interfaces.get(instruction.name).idl.members;
	oldMembers.push(...instruction.members);
	extAttrs = interfaces.get(instruction.name).idl.extAttrs;
	extAttrs.push(...instruction.extAttrs);
	break;

		* webidl2js attempts to generate informative error messages using what it knows.

		For more examples, you can check out the `test/` directory (with the generated output being in `test/__snapshots__`). Alternately, you can install [jsdom](https://www.npmjs.com/package/jsdom), [whatwg-url](https://www.npmjs.com/package/whatwg-url), or [domexception](https://www.npmjs.com/package/domexception) from npm and check out their source code. (Note that browsing them on GitHub will not work, as we do not check the generated files into Git, but instead generate them as part of publishing the package to npm.)


		### `[Reflect]`

		The `[Reflect]` extended attribute is implemented to call `this.getAttribute` or `this.setAttribute` and process the input our output using [webidl-html-reflector](https://github.com/domenic/webidl-html-reflector), generating both a getter and a setter. If `[Reflect]` is specified, the implementation class does not need to implement any getter or setter logic.


		The main module's default export is a class which you can construct with a few options:

		- `implSuffix`: a suffix used, if any, to find files within the soruce directory based on the IDL file name.

Add a README documenting everything #51

Add a README documenting everything #51

Conversation

domenic commented Aug 19, 2017

TimothyGu left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Zirro commented Aug 19, 2017 • edited Loading

Sebmaster commented Aug 19, 2017

domenic commented Aug 19, 2017

TimothyGu commented Aug 19, 2017

Zirro commented Aug 19, 2017 •

edited

Loading