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

Add Encoding Stardard by adding browser.webidl.json #300

Closed
wants to merge 3 commits into from

Conversation

saschanaz
Copy link
Contributor

@saschanaz saschanaz commented Sep 27, 2017

Adds Encoding Standard-defined types by WebIDL-generated browser.webidl.json, superceding #227.

This PR allows progressive merging of standard W3C/WHATWG specs into lib.d.ts, which means more specs can be added on top of this.

Why JSON?

XML libraries including xmldom are not being well maintained because of the transition to JSON and other formats.

Currently the JSON data is converted to existing browser.webidl.xml objects to reduce code duplication, while the ultimate goal is to replace browser.webidl.xml completely.

@saschanaz
Copy link
Contributor Author

Still too much diffs to review?

@saschanaz
Copy link
Contributor Author

Or no interest to introduce a new file?

@saschanaz
Copy link
Contributor Author

saschanaz commented Nov 7, 2017

Or what? Just too busy?

@saschanaz
Copy link
Contributor Author

Rebased, an appreciation for @DanielRosenwasser to merge from master branch 👍

@falsandtru
Copy link
Contributor

falsandtru commented Nov 21, 2017

@saschanaz You may as well split this PR into some small and clear units with leaving this PR open. In this case, maintainers can improve the project by small steps. And you can add a piece of your changes to the project. Besides, it will also reduce the diff size of main PR. For example, I sometime split my PR like this: #324, #326.

@saschanaz
Copy link
Contributor Author

This really includes Encoding Standard + WebIDL types. The latter is way smaller so probably a good point to split.

@rhysd
Copy link

rhysd commented Jan 8, 2018

What is the status of this PR? I want this for avoiding a monkey patch to Window interface in order to use TextDecoder and TextEncoder.

@mhegazy
Copy link
Contributor

mhegazy commented Jan 8, 2018

@saschanaz can you rebase please

@saschanaz
Copy link
Contributor Author

saschanaz commented Jan 8, 2018 via email

@@ -0,0 +1,381 @@
{
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need this file?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is for F# type provider to compute JSON types. Otherwise it has to read the whole file which will take more time.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks. we should name it as such then.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The current sample.xml has the same purpose, do we also want to rename it? typeprovider.xml/json?

https://github.com/Microsoft/TSJS-lib-generator/blob/048c8bd0c2d47b5c234f47a4490b2e236d11688b/TS.fsx#L59

@@ -0,0 +1,97 @@
{
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what is the source of this file?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is that one you hand authored? I am asking mainly for naming purposes. i think browser.webidl is not a good name for this one..

@saschanaz
Copy link
Contributor Author

saschanaz commented Jan 8, 2018 via email

@mhegazy
Copy link
Contributor

mhegazy commented Jan 8, 2018

then maybe we should name the generated .json file with the same name as the input webidl file. this way you can tell where it came from easily.

@saschanaz
Copy link
Contributor Author

The input files typically have the same name index.bs, which probably is not a good name.

BTW I'm merging several json files into one file browser.webidl.json, do we want to get splitted JSON files?

@mhegazy
Copy link
Contributor

mhegazy commented Jan 8, 2018

The input files typically have the same name index.bs, which probably is not a good name.

I actually meant the name of the spec. so that would be the title in this case.

BTW I'm merging several json files into one file browser.webidl.json, do we want to get splitted JSON files?

there is no reason to have them all in one.. it would be nice to be able to match the same structure that comes in from the webidl

@saschanaz
Copy link
Contributor Author

Splitted JSON files need more changes on TSJS-lib-generator as it currently assumes we have only one type definition for a name (it uses name-type mapping). In reality we have several partial definitions.

@saschanaz
Copy link
Contributor Author

saschanaz commented Jan 8, 2018

If we are to generate several d.ts files (in other words, n d.ts files for n webidl files) then it may be simpler.

@mhegazy
Copy link
Contributor

mhegazy commented Jan 9, 2018

Splitted JSON files need more changes on TSJS-lib-generator as it currently assumes we have only one type definition for a name (it uses name-type mapping). In reality we have several partial definitions.

An earlier version of this script used to do that. it did that by reading them alll, merging the input into a single json object, then processing this. we can do something similar here.

If we are to generate several d.ts files (in other words, n d.ts files for n webidl files) then it may be simpler.

that is a possibility as well..

@saschanaz
Copy link
Contributor Author

saschanaz commented Jan 9, 2018

An earlier version of this script used to do that. it did that by reading them alll, merging the input into a single json object, then processing this. we can do something similar here.

That's exactly what I'm doing on the upstream. Anyway, this PR doesn't cover any partial definitions so any merging can be a future work. I'll do the splitted JSON thing first.

@mhegazy
Copy link
Contributor

mhegazy commented Mar 7, 2018

@saschanaz with #383 in, can you try merging the .json files in.

@saschanaz
Copy link
Contributor Author

See #405 if anyone has subscribed this 😁

@saschanaz saschanaz closed this Apr 6, 2018
@saschanaz saschanaz deleted the json branch April 6, 2018 00:02
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.

4 participants