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

Definitions of NDEFMessageInit and NDEFRecordInit violates Web IDL spec #538

Open
peria opened this issue Jan 28, 2020 · 6 comments
Open

Comments

@peria
Copy link

peria commented Jan 28, 2020

Two IDL dictionaries NDEFMessageInit and NDEFRecordInit are defined as following;

dictionary   {
  required sequence<NDEFRecordInit> records;
};
dictionary NDEFRecordInit {
  ...(snip)...
  NDEFRecordDataSource data;
};
typedef (DOMString or BufferSource or NDEFMessageInit) NDEFRecordDataSource;

This reference cycle does not meet the description "The type of a dictionary member must not include the dictionary it appears on." in Web IDL spec

@reillyeon
Copy link
Member

I am curious about the reasoning behind the WebIDL specification intentionally banning recursive types. As a workaround we could define NDEFRecordDataSource as follows which should only trivially change the code developers need to write when creating embedded records,

typedef (DOMString or BufferSource or NDEFMessage) NDEFRecordDataSource;

@zolkis
Copy link
Contributor

zolkis commented Jan 28, 2020

Right, good catch. How are we supposed to define recursive structures with Web IDL then?
Use interface with 3 constructors instead of dictionary for NDEFRecord?

@peria
Copy link
Author

peria commented Jan 29, 2020

I think this issue is not urgent nor serious while most browsers are working well. :)
And I think it is a practical option to update Web IDL spec not to ban the cyclic references in dictionaries.

@rakuco
Copy link
Member

rakuco commented Sep 17, 2021

This ended up being filed again as #620 after it bit us in the Chromium implementation :/

@reillyeon
Copy link
Member

Shall we mark this as a duplicate?

@rakuco
Copy link
Member

rakuco commented Sep 17, 2021

Yeah, I think that makes sense, as all the recent discussion's tracked there.

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

No branches or pull requests

6 participants