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

Don't export most things, for mixins #55

Closed
domenic opened this issue Aug 19, 2017 · 9 comments · Fixed by #68 or #171
Closed

Don't export most things, for mixins #55

domenic opened this issue Aug 19, 2017 · 9 comments · Fixed by #68 or #171

Comments

@domenic
Copy link
Member

domenic commented Aug 19, 2017

See also #53. is/isImpl and maybe mixedInto make sense. But most of the rest of the exports don't.

To reliably identify a mixin, we can either wait for whatwg/webidl#363 or we can use a custom extended attribute in the meantime.

@TimothyGu
Copy link
Member

Are there examples of interfaces defined with NoInterfaceObject but is not a mixin?

@domenic
Copy link
Member Author

domenic commented Aug 19, 2017

A few legacy ones. E.g. https://html.spec.whatwg.org/#external

domenic added a commit that referenced this issue Aug 20, 2017
Now includes the generated wrapper class file API, guidance on writing implementation class files, and documentation for the generated utils.js file.

See #52, #53, #54, #55, and #56 for oddities discovered during the course of writing this, which we hope to clean up later. (Many of them having to do with functionality that should not be considered public, and as such is not documented here, but is exposed as if it were public.)
@TimothyGu
Copy link
Member

In that case, could we specifically flag the few NoInterfaceObject non-mixin interfaces and default to the mixin behavior for NoInterfaceObject? I could see the argument against this of course but just thought it might cause less churn.

@domenic
Copy link
Member Author

domenic commented Aug 24, 2017

I think I'd rather tag the mixins since we're going to eventually make mixins first class in Web IDL anyway so it'd be nice to get a sense of which ones they are.

@TimothyGu
Copy link
Member

Sounds just fine to me.

@ExE-Boss
Copy link
Contributor

Is this supposed to be closed?

Because README.md says that this is still an issue:

Note that all of the below are still exported for "mixin" interfaces, but only `isImpl()` and `is()` make sense ([#55](https://github.com/jsdom/webidl2js/issues/55)).

Also, #68’s description says that it doesn’t fix this.

@TimothyGu
Copy link
Member

Ugh, GitHub's auto-closed this issue even though I said:

Note, this does not yet fix #55

Thanks for the note @ExE-Boss!

@TimothyGu TimothyGu reopened this Sep 19, 2019
@ExE-Boss
Copy link
Contributor

Yeah, that’s because you wrote “fix” followed by the issue number separated by a single space (U+0020), which is all GitHub cares about.

@TimothyGu
Copy link
Member

Fixed in #135. Interface mixins don’t cause files to be generated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants