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

performance.measure() overloads instead of optional arguments #1239

Open
lgodziejewski opened this issue Jan 4, 2022 · 1 comment · May be fixed by #1280
Open

performance.measure() overloads instead of optional arguments #1239

lgodziejewski opened this issue Jan 4, 2022 · 1 comment · May be fixed by #1280

Comments

@lgodziejewski
Copy link

lgodziejewski commented Jan 4, 2022

Hey there

according to MDN Docs, the performance.measure() fn has 4 different argument sets (overloads) - the current typing suggests only one and marks some of the args as optional.

Perhaps it'd be better to have the 4 overloads listed explicitly? i.e. instead of:

measure(measureName: string, startOrMeasureOptions?: string | PerformanceMeasureOptions, endMark?: string): PerformanceMeasure;

have sth like:

measure(measureName: string): PerformanceMeasure
measure(measureName: string, MeasureOptions: PerformanceMeasureOptions): PerformanceMeasure
measure(measureName: string, startMark: string): PerformanceMeasure
measure(measureName: string, startMark?: string, endMark: string): PerformanceMeasure

WDYT? This would enable better autocomplete suggestions in code editors (notice the 1/3 on the 2nd pic):

when pic
before image
after image

If it's OK for you - what would be the best way to submit these changes?

@yume-chan yume-chan linked a pull request Mar 2, 2022 that will close this issue
@yume-chan
Copy link
Contributor

Your fourth overload is invalid, it has a required argument after an optional argument.

I did the conversion in #1280, basically the steps are:

  1. Fork and clone this repo
  2. Find out what kind the type you want to modify
    1. Find out which spec the type belongs to. Usually MDN page has a "Specifications" section.
    2. Scroll to the IDL part, check whether it's an interface, a dictionary or something else like a callback.
  3. Open inputfiles/overridingTypes.jsonc, search for existed entries for your type, if nothing found, add a new entry in the corresponding section (interface, dictionary, etc.)
  4. Check how other overrides work, write your own one (may be difficult, there is no docs)
  5. Run npm install, npm build, npm baseline-accept, npm run test
  6. Check the output in baselines is what you want, and there is no test errors.

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 a pull request may close this issue.

2 participants