protocol-json-as ts generates typed json file

[nojvek]

@themaxdavitt he is my proposed approach ^ wdyt?

also @paulirish (just wanna add this on your radar)

Found a couple of changes to protocol schema and updated them.

[googlebot]

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

[themaxdavitt]

@googlebot I consent.

[googlebot]

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

[themaxdavitt]

@nojvek I actually thought of a few more tweaks we could make to this PR before it gets accepted, see here: https://github.com/themaxdavitt/devtools-protocol/commit/83a994e3dbc12e998af6df8a19db151a48797444 (it's on a separate branch)

Here's an example of what it could enable:

// still allows importing from 'devtools-protocol/types/...'
import defs, { ProtocolSchema } from 'devtools-protocol';

// just to test that types are being re-exported correctly
const test: ProtocolSchema.Definition = defs.browser;

console.log(test);

Basically, it's just reorganizing the types a little bit and exporting the JSON from the repository (using our newly exported types). What do you think?

[nojvek]

Will take a look at this Monday. Although I don’t work at @google. We need @paulirish to review and merge if this is going to land.

[nojvek]

It's been many months. I'm not sure whether @paulirish or any of the devtools authors will take this change anytime soon. I'll wait another week and close out this PR.

I personally like to keep my PR list clean. Either something gets accepted or rejected in a timely manner, limbo state is the worst. It causes anxiety to me.

[TimvdLippe]

Sorry for the lack of response, I missed this PR. It's not clear what the benefit of this change is, given that we already publish .d.ts files. It seems like this change increases the infrastructure complexity significantly, but without a clear upside (at least to me). Could you please add a PR description that describes the use case, what issues it would resolve and why alternative solutions are not sufficient? That would give me additional information to make a decision, thanks 😄

[themaxdavitt]

Sorry for the late response also! :)

Essentially, this PR was to type-check the JSON files used to generate typings (see https://github.com/ChromeDevTools/devtools-protocol/pull/211 for more discussion). It looks like there will be some merge conflicts if we decide to push this now though - I'm not as invested in seeing it through as I was earlier in the year so it's your call.

[nojvek]

@TimvdLippe

The whole idea was that the protocol.json files that come from upstream google sometimes have incorrect types. This PR would ensure that the prototocol.json files map to the protoco.d.ts schema.

When I was maintaining https://github.com/nojvek/chrome-remote-debug-protocol (which now is now merged in this repo), I noticed is that occasionally the protocol.json files would send a number instead of a string and break the generator. I'd occasionally send fixes to chromium repo. I'm not sure now if Google now has some validation in the chromium repo that checks against some schema.

But without any form of schema validation on protocol.json files, we're a bit blind.

[TimvdLippe]

Ah I see what you mean. In that case, we should fix that upstream where we have the protocol definition. Iiuc, we have some presubmit checks and I think these need to be expanded to prevent such issues? https://source.chromium.org/chromium/chromium/src/+/master:third_party/inspector_protocol/check_protocol_compatibility.py;l=1;drc=0ddc5e583a055706fba006b1f75d50c14119036d?originalUrl=https:%2F%2Fcs.chromium.org%2F

[nojvek]

Closing because chromium needs to fix it upstream.