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

Bring code generation into this repo, update to latest specs, support async functions natively #10

Merged
merged 124 commits into from
May 2, 2022

Conversation

j-f1
Copy link
Member

@j-f1 j-f1 commented Mar 30, 2022

I rebuilt the code generation using the official (JS-based) WebIDL parser. Taking inspiration from webidl2swift, this will hopefully improve on that package by not having to maintain the parser component. I also:

  • Added async versions of Promise-returning functions
  • Added enormous numbers of Web APIs that were previously not included
    • both existing and new APIs! The full list of included specs is in IDLBuilder, and it would be great to expand that.
  • Expanded support for wrapped closure properties (using code generation for now, until we get variadic generics)
  • Moved the code generation pipeline into this repo, to make it easier to make changes

TODOs:

  • Add back support for union types
    • This will probably happen by calculating the Cartesian product of all union-typed function parameters, to avoid unnecessary user-side ceremony
    • Union return types (if present) will still have to be enums, I think, though.
  • Add back support for type-erased protocol types (AnyParentNode)
    • mark parameters as taking the protocol rather than the enum
  • Rewrite dictionaries to not subclass JSObject because that is bad
  • Do something about the fact that JS can run in different contexts (e.g. window-side code vs the plethora of worker types)
  • Implement iterators and async iterators properly
    • this requires upstream changes to JavaScriptKit that I will be tackling later to enable usage of Symbol keys
  • add support for readonly closure properties
  • re-evaluate entries in the ignored dictionary in IDLBuilder, and make as many APIs as possible work (or manually re-implement them)
  • Add and error-check remaining web APIs
  • test with real code
  • configure GitHub Actions to periodically update the generated code

@j-f1
Copy link
Member Author

j-f1 commented Apr 23, 2022

Ok, I think this is the minimal set of specs that can be included.

@j-f1 j-f1 marked this pull request as ready for review April 23, 2022 00:42
@MaxDesiatov
Copy link
Contributor

MaxDesiatov commented Apr 30, 2022

Currently seeing some breakage in RequestInit.swift and Response.swift 🤔

@MaxDesiatov
Copy link
Contributor

Ready for review? Or should we merge as is and then tackle separate APIs on case-by-case basis as we have more experience using them?

@j-f1
Copy link
Member Author

j-f1 commented May 2, 2022

I guess merging as-is makes sense.

@j-f1 j-f1 changed the title [WIP] Bring code generation into this repo, update to latest specs, support async functions natively Bring code generation into this repo, update to latest specs, support async functions natively May 2, 2022
@MaxDesiatov MaxDesiatov added the enhancement New feature or request label May 2, 2022
@MaxDesiatov
Copy link
Contributor

Ugh, even opening the diff screen slows the browser page down to a crawl. Merging as-is indeed 😅

@MaxDesiatov MaxDesiatov merged commit 45ed26d into main May 2, 2022
@MaxDesiatov MaxDesiatov deleted the js-parse branch May 2, 2022 10:14
@kateinoigakukun
Copy link
Member

Can we remove auto-generated sources from repo and use SwiftPM built-tool plugin instead?

@MaxDesiatov
Copy link
Contributor

I hope we can, and it's more of a question of whether we want to make it more convenient for DOMKit devs or our users. Adding an additional codegen step will slow down the build, but OTOH we could speed that up in the future by providing prebuilt binaries, similar to what Linux already supports. Obviously, with a caveat for the lack of ABI stability when targeting Wasm.

Let's investigate SwiftPM codegen plugins and see how that performs then?

@j-f1
Copy link
Member Author

j-f1 commented May 2, 2022

FWIW the codegen of the full set of specs takes under 10 seconds on my M1 Pro

@MaxDesiatov
Copy link
Contributor

The downside of doing codegen on user's side is that we'd require Node.js to be installed for all users of DOMKit. What do you think about this new potential requirement?

@kateinoigakukun
Copy link
Member

For the build speed, it's reasonable to provide pre-generated sources in this repo 👍
BTW, we still have some benefits to providing the codegen plugin to allow users to try the latest WebIDL without waiting for DOMKit update.

@kateinoigakukun
Copy link
Member

Let me try porting it to a plugin

@MaxDesiatov
Copy link
Contributor

I'm still not 100% sure that passing build flags is convenient enough with SwiftPM, but maybe we could switch between plugin and pre-generated versions with a build flag after all?

@j-f1
Copy link
Member Author

j-f1 commented May 2, 2022

we'd require Node.js to be installed for all users of DOMKit

Not necessarily — as part of our release process, we could bundle the output of node parse-all.js as a JSON file. That could then be turned into Swift code at user build time with no Node.js requirement.

@MaxDesiatov
Copy link
Contributor

Ah, I missed that part, that's great!

@kateinoigakukun
Copy link
Member

Hmm, I faced some troubles while porting to the plugin system.

  1. Output filenames of WebIDLToSwift cannot be determined at build planning time, so we need to use .prebuildCommand
  2. prebuildCommand doesn't support build tools built from sources for now due to limitations of SwiftPM build planning architecture.

If we complete #14, it would be able to use .buildCommand, I think.

@MaxDesiatov
Copy link
Contributor

MaxDesiatov commented May 2, 2022

Does this mean that we have to know all the names of .swift output files upfront for WebIDLToSwift to work as .buildCommand?

@kateinoigakukun
Copy link
Member

kateinoigakukun commented May 2, 2022

Does this mean that we have to know all the names of .swift output files upfront for WebIDLToSwift to work as .buildCommand?

Right

@j-f1
Copy link
Member Author

j-f1 commented May 2, 2022

That could be handled by emitting all the sources into one file — I had the code emit to multiple files to match the old behavior, but there’s no particular reason for that.

@kateinoigakukun
Copy link
Member

OK, let me try merging into a single file

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants