Closed j-f1 closed 2 years ago
Given that the PR in its current state is huge, do you think that we could start with one small change here and review/merge it separately? I'd assume everything else in the list is blocked by codegen, could we start with moving that to a separate PR?
It would definitely be possible to start by removing as many of the spec files as possible (while still avoiding compile errors caused by missing declarations). Most of the added files are from specs not previously included in DOMKit.
Ok, I think this is the minimal set of specs that can be included.
Currently seeing some breakage in RequestInit.swift
and Response.swift
🤔
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?
I guess merging as-is makes sense.
Ugh, even opening the diff screen slows the browser page down to a crawl. Merging as-is indeed 😅
Can we remove auto-generated sources from repo and use SwiftPM built-tool plugin instead?
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?
FWIW the codegen of the full set of specs takes under 10 seconds on my M1 Pro
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?
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.
Let me try porting it to a plugin
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?
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.
Ah, I missed that part, that's great!
Hmm, I faced some troubles while porting to the plugin system.
WebIDLToSwift
cannot be determined at build planning time, so we need to use .prebuildCommand
prebuildCommand
doesn't support build tools built from sources for now due to limitations of SwiftPM build planning architecture.If we complete https://github.com/swiftwasm/DOMKit/issues/14, it would be able to use .buildCommand
, I think.
Does this mean that we have to know all the names of .swift
output files upfront for WebIDLToSwift
to work as .buildCommand
?
Does this mean that we have to know all the names of .swift output files upfront for WebIDLToSwift to work as .buildCommand?
Right
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.
OK, let me try merging into a single file
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:async
versions of Promise-returning functionsIDLBuilder
, and it would be great to expand that.TODOs:
AnyParentNode
)JSObject
because that is badignored
dictionary inIDLBuilder
, and make as many APIs as possible work (or manually re-implement them)