Skip to content

Inherit const members in constructors #532

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

Merged
merged 3 commits into from
Jul 11, 2018

Conversation

saschanaz
Copy link
Contributor

@saschanaz saschanaz commented Jul 11, 2018

Fixes #531

This doesn't look very ideal as this basically copies everything, but not sure what should be done for those inline type literals.

Maybe we can introduce new (InterfaceName)Constants interfaces and use them:

declare var MyClass: {
  prototype: MyClass;
  new(): MyClass;
} & MyClassConstants;

What do you think? @mhegazy

@mhegazy
Copy link
Contributor

mhegazy commented Jul 11, 2018

that looks like a lot of duplication really. Adding a new interface per variable does not seem much better either with the global scope pollution; the intersection does not look attractive either.

I would say the issue here is that these all should be defined as classes instead of variable+interface pairs, this way you get the static inheritance for free.

I suggest we shelve this until we have updated the DOM to use the webidl as a source first, then revisit the class generation question.

@saschanaz
Copy link
Contributor Author

saschanaz commented Jul 11, 2018

I suggest we shelve this until we have updated the DOM to use the webidl as a source first

Actually this becomes a blocker when WebGL has moved every constant from interface WebGLRenderingContext to interface mixin WebGLRenderingContextBase. This way the constructor loses all constants when without static inheritance.

Should we just add a special check for WebGLRenderingContext case as a temporary solution?

Edit: Classes won't help the WebGL case as mixins are not classes.
Edit2: Hmm, maybe we can generally copy the constants for mixins (as IMO currently it's only for the WebGL case), and defer others for now.

@saschanaz
Copy link
Contributor Author

This PR now only covers mixin cases.

@mhegazy mhegazy merged commit 68fe8bb into microsoft:master Jul 11, 2018
@saschanaz saschanaz deleted the inherit-const branch July 12, 2018 04:05
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 this pull request may close these issues.

2 participants