Skip to content

Conversation

@Bashamega
Copy link
Contributor

@Bashamega Bashamega commented Sep 29, 2025

Migrate to be using the json API
closes #2151
closes #2086
closes #2078
closes #2209

@Bashamega
Copy link
Contributor Author

I have updated it @saschanaz,
But the import JSON file didn't work for some reason.
This is the error: https://github.com/microsoft/TypeScript-DOM-lib-generator/actions/runs/18124591077/job/51576830796?pr=2171

Copy link
Contributor

@saschanaz saschanaz left a comment

Choose a reason for hiding this comment

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

I think you reverted too early. The module error doesn't happen locally.

| (startswith("/en-us/docs/web/api/") or startswith("/en-us/docs/webassembly/reference/javascript_interface/")))
)
| { mdn_url, pageType, summary } ] }' \
> mdn.json
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 bash magic when we can filter it in the script instead?

@Bashamega
Copy link
Contributor Author

The module error also occurs on my local machine (Ubuntu), @saschanaz.

I used the bash magic because it significantly reduces the output size — most of the data isn’t needed.
For comparison:

Current (filtered and formatted): 2.5 MB

Original (unfiltered, unformatted): 28.7 MB

That’s over 11× larger (28.7 MB vs 2.5 MB), so I think the extra shell step is worth it.
Plus, the command is still quite readable.

@Bashamega
Copy link
Contributor Author

I’ve added support for _static, so there is no missing comments now, @jakebailey.

@saschanaz
Copy link
Contributor

saschanaz commented Oct 1, 2025

The module error also occurs on my local machine (Ubuntu), @saschanaz.

Are you seeing the exact same module error? Because that error says you don't have lib/build.js, which means typescript silently failed to compile anything at all. Can you check what's in lib/ in that case?

I used the bash magic because it significantly reduces the output size — most of the data isn’t needed. For comparison:

Current (filtered and formatted): 2.5 MB

Original (unfiltered, unformatted): 28.7 MB

That’s over 11× larger (28.7 MB vs 2.5 MB), so I think the extra shell step is worth it. Plus, the command is still quite readable.

Hmm, fair. I think we should still fetch and filter in typescript instead, as bash usage would break on Windows.

@saschanaz
Copy link
Contributor

Or well, in JS, to save a build step.

@Bashamega
Copy link
Contributor Author

Bashamega commented Oct 1, 2025

Hmm, fair. I think we should still fetch and filter in typescript instead, as bash usage would break on Windows.

I don't think that this is an issue, since no one would actually run it, but sure, I will write a script to generate the file

@Bashamega
Copy link
Contributor Author

Are you seeing the exact same module error? Because that error says you don't have lib/build.js, which means typescript silently failed to compile anything at all. Can you check what's in lib/ in that case?
Yes, it is the same error

@saschanaz
Copy link
Contributor

Can you check what's in lib/ in that case?

@Bashamega
Copy link
Contributor Author

image Here

@jakebailey
Copy link
Member

How are you able to apply changes to the PR?

@saschanaz
Copy link
Contributor

How are you able to apply changes to the PR?

With a hack - @Bashamega also opened saschanaz#241 and that grants me some sort of access. But only through GitHub UI and no direct git push.

@jakebailey
Copy link
Member

Interesting, did not know that GitHub allowed a branch to be open to two PRs even across repos.

@Bashamega
Copy link
Contributor Author

Thanks for helping @saschanaz, I will pull and fix the code right now, give me 5-10 minutes

@saschanaz
Copy link
Contributor

Thanks for helping @saschanaz, I will pull and fix the code right now, give me 5-10 minutes

Cool, I'll stop making changes until then.

@Bashamega
Copy link
Contributor Author

I have updated the MDN file @saschanaz
Is there anything else you want me to update?

@Bashamega
Copy link
Contributor Author

How are you able to apply changes to the PR?

With a hack - @Bashamega also opened saschanaz#241 and that grants me some sort of access. But only through GitHub UI and no direct git push.

I think the best thing to solve this issue with permission to invite @saschanaz as a collaborator for this repository :)

@saschanaz
Copy link
Contributor

Apparently Microsoft didn't like that idea 🥺

@jakebailey
Copy link
Member

I have a plan to fix the old workflow; I think I just over-classified the repo which locked out some things. (Will take a bit for the system to update repo rules.)

@Bashamega
Copy link
Contributor Author

I hope you can gain access

@Bashamega
Copy link
Contributor Author

Also, for the future if you don't mind @saschanaz
Do you have a faster way of communicating for example, Discord or Slack?

@jakebailey
Copy link
Member

There's a dom lib channel on the discord, fwiw

@jakebailey
Copy link
Member

Everything in order now?

(This is a maintained tagged PR so I have to merge it anyway, just let me know)

@Bashamega
Copy link
Contributor Author

There's a dom lib channel on the discord, fwiw

I know but I think @saschanaz is not on discord or inactive

@saschanaz
Copy link
Contributor

I think mostly fine but I'll check again in 30min

@saschanaz
Copy link
Contributor

Actually, LGTM

@github-actions
Copy link
Contributor

github-actions bot commented Nov 1, 2025

Sorry @saschanaz, you don't have access to these files:

@Bashamega
Copy link
Contributor Author

Thanks

@jakebailey
Copy link
Member

I'll check back then if I'm still awake (my fire alarm went off in the middle of the night, I'm not normally around at this time...)

@jakebailey
Copy link
Member

Oh, good

@jakebailey jakebailey merged commit d21b002 into microsoft:main Nov 1, 2025
7 checks passed
@Bashamega Bashamega deleted the mdn-v2 branch November 1, 2025 10:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

don't include js-nolint in tooltips Migrate to MDN metadata.json

4 participants