Skip to content

Add more comments#1993

Merged
github-actions[bot] merged 81 commits into
microsoft:mainfrom
Bashamega:descriptions
Jun 3, 2025
Merged

Add more comments#1993
github-actions[bot] merged 81 commits into
microsoft:mainfrom
Bashamega:descriptions

Conversation

@Bashamega

Copy link
Copy Markdown
Contributor

related to #1940
Hello:)
In my last PR I have used MDN to generate comments for interface declarations, in this pr I add it for the properties.

@github-actions

Copy link
Copy Markdown
Contributor

Thanks for the PR!

This section of the codebase is owned by @saschanaz - if they write a comment saying "LGTM" then it will be merged.

@Bashamega

Bashamega commented Apr 23, 2025

Copy link
Copy Markdown
Contributor Author

Not ready for review
Ready for review

@saschanaz saschanaz left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'm yet to review this, but please do not convert to sync APIs.

@Bashamega

Copy link
Copy Markdown
Contributor Author

I'm yet to review this, but please do not convert to sync APIs.

Sure

@Bashamega

Bashamega commented Apr 23, 2025

Copy link
Copy Markdown
Contributor Author

Hello @saschanaz
I have switched the old functions to async, but the new function is still sync because the emitter is not async. I tried switching it to async, but it broke.

Comment thread baselines/audioworklet.iterable.generated.d.ts
Comment thread src/build/emitter.ts Outdated
Comment thread src/build/mdn-comments.ts Outdated
@Bashamega

Copy link
Copy Markdown
Contributor Author

I have updated the context to not call the API in the emitter, but that cause the test to fail, I will fix it later

@Bashamega

Copy link
Copy Markdown
Contributor Author

Done @saschanaz

Comment thread src/build/emitter.ts Outdated
Comment thread src/build.ts Outdated
Comment thread src/build/mdn-comments.ts Outdated
Comment thread src/build/mdn-comments.ts Outdated
@saschanaz

Copy link
Copy Markdown
Contributor

LGTM 🎉

@github-actions

github-actions Bot commented Jun 3, 2025

Copy link
Copy Markdown
Contributor

There was an issue merging, maybe try again saschanaz. Details

@Bashamega

Copy link
Copy Markdown
Contributor Author

Thank you very much @saschanaz

@jakebailey jakebailey left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

After this, there's nothing else to comment on, right? This is as large as we're going to get?

Each one of these makes parsing the DOM types slower, so I'm hoping that we stop at some point 😅

@saschanaz

Copy link
Copy Markdown
Contributor

Technically we still have events, but adding comments there would be a bit awkward as that would only work for onevent attributes and not for addEventListener calls. Probably fair to just skip that.

@saschanaz

Copy link
Copy Markdown
Contributor

LGTM

@github-actions github-actions Bot merged commit 5bd485e into microsoft:main Jun 3, 2025
6 checks passed
@github-actions

github-actions Bot commented Jun 3, 2025

Copy link
Copy Markdown
Contributor

Merging because @saschanaz is a code-owner of all the changes - thanks!

@Bashamega

Bashamega commented Jun 4, 2025

Copy link
Copy Markdown
Contributor Author

Hello @jakebailey
First of all, thank you very much for merging this pr.
Secondly, I think we can improve the compiler's speed by caching comments in a JSON file, which will serve as the comments reference. The GitHub action will be responsible for updating it. If this sound good to you, I can draft a pr for it and compare the speed.
Also, there are still some missing comments.

@jakebailey

Copy link
Copy Markdown
Member

I don't know what you mean, sorry. The comments have to be in a declaration file. Moving them elsewhere won't speed things up (and will make the compiler/editor stuff challenging).

@Bashamega

Copy link
Copy Markdown
Contributor Author

I don't know what you mean, sorry. The comments have to be in a declaration file. Moving them elsewhere won't speed things up (and will make the compiler/editor stuff challenging).

I mean instead of fetching the data each time you run the generate command, we can put the comments in a json file and the builder will use that json file instead of fetching the summaries every time

@HolgerJeromin

HolgerJeromin commented Jun 4, 2025

Copy link
Copy Markdown
Contributor

I am pretty sure Jake meant the usage (in the editor and compile of user code) of the generated files will slow down the typescript compiler as it has to parse more data.
Build/generation time of these files is uninteresting.
tsdoc belongs into the .d.ts file and not in a (faster?) parallel world which needs to be invented and maintained.

@Bashamega Bashamega deleted the descriptions branch November 3, 2025 04:18
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.

5 participants