Skip to content

[@types/carbon-components-react] Bump to 7.6, type updates & fixes.#38983

Merged
armanio123 merged 12 commits intoDefinitelyTyped:masterfrom
kalbert312:ccr-fixes-2019-10-08
Oct 8, 2019
Merged

[@types/carbon-components-react] Bump to 7.6, type updates & fixes.#38983
armanio123 merged 12 commits intoDefinitelyTyped:masterfrom
kalbert312:ccr-fixes-2019-10-08

Conversation

@kalbert312
Copy link
Copy Markdown
Contributor

@kalbert312 kalbert312 commented Oct 8, 2019

Please fill in this template.

  • Use a meaningful title for the pull request. Include the name of the package modified.
  • Test the change in your own code. (Compile and run.)
  • Add or edit tests to reflect the change. (Run with npm test.)
  • Follow the advice from the readme.
  • Avoid common mistakes.
  • Run npm run lint package-name (or tsc if no tslint.json is present).

Select one of these and delete the others:

If changing an existing definition:

Comment on lines +15 to +17
menuOffset?:
MenuOffsetData
| ((menuBody: HTMLElement, menuDirection: TooltipProps['direction']) => Required<MenuOffsetData> | undefined);
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Note: The linter was producing bad code for this. I was getting menuOffset?: | MenuOffsetData | (parenthesized fn)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Now I'm wondering if it's because of the non-parenthesized return result of the fn....

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Just checked. Running git hook on

    menuOffset?:
        MenuOffsetData
        | ((menuBody: HTMLElement, menuDirection: TooltipProps['direction']) => (Required<MenuOffsetData> | undefined));

produced

    menuOffset?:
        | MenuOffsetData
        | ((menuBody: HTMLElement, menuDirection: TooltipProps['direction']) => Required<MenuOffsetData> | undefined);

@typescript-bot typescript-bot added the Author is Owner The author of this PR is a listed owner of the package. label Oct 8, 2019
@typescript-bot
Copy link
Copy Markdown
Contributor

@kalbert312 Thank you for submitting this PR!

Pull requests from definition owners are typically merged after quick review from a DefinitelyTyped maintainer once the CI passes.

In the meantime, if the build fails or a merge conflict occurs, I'll let you know. Have a nice day!

@typescript-bot
Copy link
Copy Markdown
Contributor

Since you're a listed owner and the build passed, this PR is fast-tracked. A maintainer will merge shortly. If it shouldn't be merged yet, please leave a comment saying so and we'll wait. Thank you for your contribution to DefinitelyTyped!

@typescript-bot
Copy link
Copy Markdown
Contributor

👋 Hi there! I’ve run some quick performance metrics against master and your PR. This is still an experiment, so don’t panic if I say something crazy! I’m still learning how to interpret these metrics.

Let’s review the numbers, shall we?

master #38983 diff
Batch compilation
Memory usage (MiB) 153.3 152.3 -0.6%
Type count 58058 58713 +1.1%
Assignability cache size 61440 62008 +0.9%
Subtype cache size 61 61 0.0%
Identity cache size 12 12 0.0%
Language service
Samples taken 256 331 +29.3%
Identifiers in tests 256 331 +29.3%
getCompletionsAtPosition
    Mean duration (ms) 814.1 985.9 +21.1% 🔸
    Median duration (ms) 814.3 909.2 +11.7%
    Mean CV 5.3% 6.3% +19.8%
    Worst duration (ms) 933.4 1218.7 +30.6% 🔸
    Worst identifier rowProps React
getQuickInfoAtPosition
    Mean duration (ms) 750.9 933.1 +24.3% 🔸
    Median duration (ms) 740.7 878.4 +18.6%
    Mean CV 5.9% 7.1% +19.5%
    Worst duration (ms) 980.0 1214.2 +23.9% 🔸
    Worst identifier getHeaderProps cells
System information
Node version v10.16.3 v10.16.3
CPU count 2 2
CPU speed 2.294 GHz 2.397 GHz
CPU model Intel(R) Xeon(R) CPU E5-2673 v4 @ 2.30GHz Intel(R) Xeon(R) CPU E5-2673 v3 @ 2.40GHz
CPU Architecture x64 x64
Memory 6.8 GiB 6.8 GiB
Platform linux linux
Release 4.15.0-1055-azure 4.15.0-1059-azure

First off, note that the system varied slightly between these two runs, so you’ll have to take these measurements with a grain of salt.

It looks like there are several metrics that changed quite a bit. You might want to take a look and make sure your changes won’t cause slow-downs for users consuming these types.


If you have any questions or comments about me, you can ping @andrewbranch. Have a nice day!

@armanio123 armanio123 merged commit d56b018 into DefinitelyTyped:master Oct 8, 2019
@typescript-bot
Copy link
Copy Markdown
Contributor

I just published @types/carbon-components-react@7.6.0 to npm.

alexanderson1993 pushed a commit to alexanderson1993/DefinitelyTyped that referenced this pull request Oct 11, 2019
…efinitelyTyped#38983)

* Bump to 7.6.

* Fix missing required id prop.

* Update InlineLoading.

* Update ToolbarSearch.

* Update Tooltip.

* Update SideNav props.

* Update Notification.

* Improve DataTable types.

* Lint fix.

* Fix extra new line.

* Reverting children type change on SideNav components due to carbon issue.

* Lint fix.
chivesrs pushed a commit to chivesrs/DefinitelyTyped that referenced this pull request Nov 19, 2019
…efinitelyTyped#38983)

* Bump to 7.6.

* Fix missing required id prop.

* Update InlineLoading.

* Update ToolbarSearch.

* Update Tooltip.

* Update SideNav props.

* Update Notification.

* Improve DataTable types.

* Lint fix.

* Fix extra new line.

* Reverting children type change on SideNav components due to carbon issue.

* Lint fix.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Author is Owner The author of this PR is a listed owner of the package.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants