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.

Sign up for GitHub

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

Jump to bottom

@types/draft-js fix mixed immutable version problem #36402

Merged
merged 2 commits into from
Jun 26, 2019
Merged

@types/draft-js fix mixed immutable version problem #36402

merged 2 commits into from
Jun 26, 2019

Conversation

MunifTanjim
Copy link
Contributor

@MunifTanjim MunifTanjim commented Jun 24, 2019 •
edited

Package Dependency Dependency Semver
draft-js immutable ~3.7.4
@types/draft-js immutable ^3.8.1

This causes the following error:

The inferred type of '...' cannot be named without a reference to 'draft-js/node_modules/immutable'. This is likely not portable. A type annotation is necessary.ts(2742)

Relatated issue details: microsoft/TypeScript#29808

This PR aims to solve that by using the semver ~3.7.4 for immutable in @types/draft-js.

I also had to bump the TypeScript Version to 2.9 for the following packages:

  • @types/draft-js
  • @types/braft-editor
  • @types/draft-convert
  • @types/react-draft-wysiwyg
  • @types/react-rte

I don't know if there's any better way to resolve this. I'm open to suggestions.

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:

  • Provide a URL to documentation or source code which provides context for the suggested changes: <>
  • Increase the version number in the header if appropriate.
  • If you are making substantial changes, consider adding a tslint.json containing { "extends": "dtslint/dt.json" }.

@typescript-bot typescript-bot added this to Needs Author Attention in Pull Request Status Board Jun 24, 2019
@typescript-bot typescript-bot added Popular package This PR affects a popular package (as counted by NPM download counts). The Travis CI build failed labels Jun 24, 2019
@typescript-bot
Copy link
Contributor

typescript-bot commented Jun 24, 2019 •
edited

@MunifTanjim Thank you for submitting this PR!

🔔 @petitspois @dmitryrogozhny @eelco @ghotiphud @schwers @michael-yx-wu @willisplummer @smvilar @sulf @pablopunk @claudiopro @khawkinson @imechZhangLY @brunoMaurice @ldanet @jclyons52 - please review this PR in the next few days. Be sure to explicitly select Approve or Request Changes in the GitHub UI so I know what's going on.

If no reviewer appears after a week, a DefinitelyTyped maintainer will review the PR instead.

@typescript-bot
Copy link
Contributor

@MunifTanjim The Travis CI build failed! Please review the logs for more information.

Once you've pushed the fixes, the build will automatically re-run. Thanks!

@typescript-bot typescript-bot moved this from Needs Author Attention to Waiting for Reviewers in Pull Request Status Board Jun 24, 2019
@typescript-bot
Copy link
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?

braft-editor/v1

Comparison details for braft-editor/v1 📊
master #36402 diff
Batch compilation
Type count 53906 53845 -0.1%
Assignability cache size 53155 53120 -0.1%
Subtype cache size 7 7 0.0%
Identity cache size 8 8 0.0%
Language service
Samples taken 175 35 -80.0%
Identifiers in tests 35 35 0.0%
getCompletionsAtPosition
    Mean duration (ms) 657.4 556.6 -15.3%
    Median duration (ms) 627.1 551.0 -12.1%
    Mean CV 8.6%
    Worst duration (ms) 858.5 736.8 -14.2%
    Worst identifier BraftEditor div
getQuickInfoAtPosition
    Mean duration (ms) 561.6 502.1 -10.6%
    Median duration (ms) 537.3 495.1 -7.9%
    Mean CV 12.1%
    Worst duration (ms) 661.1 619.5 -6.3%
    Worst identifier handleChange BraftEditor

It looks like nothing changed too much. I’m pretty lenient since I’m still an experiment, so take a look anyways and make sure nothing looks out of place.

draft-convert/v2

Comparison details for draft-convert/v2 📊
master #36402 diff
Batch compilation
Memory usage 127079840.0 126902808.0 -0.1%
Type count 53524 53460 -0.1%
Assignability cache size 53050 53008 -0.1%
Subtype cache size 0 0
Identity cache size 5 5 0.0%
Language service
Samples taken 13 13 0.0%
Identifiers in tests 13 13 0.0%
getCompletionsAtPosition
    Mean duration (ms) 552.5 548.0 -0.8%
    Median duration (ms) 560.2 554.8 -1.0%
    Mean CV 8.2% 8.7% +6.4%
    Worst duration (ms) 581.1 587.6 +1.1%
    Worst identifier draftConvert draftConvert
getQuickInfoAtPosition
    Mean duration (ms) 480.3 490.1 +2.0%
    Median duration (ms) 478.5 493.9 +3.2%
    Mean CV 8.0% 9.9% +23.9%
    Worst duration (ms) 505.9 515.5 +1.9%
    Worst identifier html draftConvert

It looks like nothing changed too much. I’m pretty lenient since I’m still an experiment, so take a look anyways and make sure nothing looks out of place.

draft-js/v*

Comparison details for draft-js/v* 📊
master #36402 diff
Batch compilation
Memory usage 135120440.0 142455592.0 +5.4%
Type count 52032 51986 -0.1%
Assignability cache size 51339 51327 0.0%
Subtype cache size 38 38 0.0%
Identity cache size 16 16 0.0%
Language service
Samples taken 500 500 0.0%
Identifiers in tests 500 500 0.0%
getCompletionsAtPosition
    Mean duration (ms) 561.4 563.3 +0.4%
    Median duration (ms) 553.8 556.8 +0.5%
    Mean CV 6.9% 6.9% +0.4%
    Worst duration (ms) 703.8 710.6 +1.0%
    Worst identifier label blockType
getQuickInfoAtPosition
    Mean duration (ms) 499.7 504.5 +1.0%
    Median duration (ms) 494.4 497.6 +0.6%
    Mean CV 8.1% 8.0% -1.0%
    Worst duration (ms) 623.2 651.3 +4.5%
    Worst identifier label state

It looks like nothing changed too much. I’m pretty lenient since I’m still an experiment, so take a look anyways and make sure nothing looks out of place.

react-draft-wysiwyg/v1

Comparison details for react-draft-wysiwyg/v1 📊
master #36402 diff
Batch compilation
Memory usage 146450720.0 159989696.0 +9.2%
Type count 55187 55126 -0.1%
Assignability cache size 54018 53976 -0.1%
Subtype cache size 20 20 0.0%
Identity cache size 8 8 0.0%
Language service
Samples taken 201 201 0.0%
Identifiers in tests 201 201 0.0%
getCompletionsAtPosition
    Mean duration (ms) 568.0 570.8 +0.5%
    Median duration (ms) 567.4 567.0 -0.1%
    Mean CV 7.8% 7.7% -1.2%
    Worst duration (ms) 684.6 656.6 -4.1%
    Worst identifier contentState h3
getQuickInfoAtPosition
    Mean duration (ms) 500.0 504.2 +0.8%
    Median duration (ms) 496.4 495.8 -0.1%
    Mean CV 8.4% 8.5% +1.9%
    Worst duration (ms) 604.6 630.4 +4.3%
    Worst identifier defaultContentState onContentStateChange

It looks like nothing changed too much. I’m pretty lenient since I’m still an experiment, so take a look anyways and make sure nothing looks out of place.

react-rte/v*

Comparison details for react-rte/v* 📊
master #36402 diff
Batch compilation
Memory usage 145648248.0 145427600.0 -0.2%
Type count 53857 53793 -0.1%
Assignability cache size 53148 53106 -0.1%
Subtype cache size 6 6 0.0%
Identity cache size 8 8 0.0%
Language service
Samples taken 32 32 0.0%
Identifiers in tests 32 32 0.0%
getCompletionsAtPosition
    Mean duration (ms) 560.1 558.1 -0.4%
    Median duration (ms) 562.6 551.7 -1.9%
    Mean CV 11.3% 10.7% -5.4%
    Worst duration (ms) 650.9 659.9 +1.4%
    Worst identifier RichTextEditor React
getQuickInfoAtPosition
    Mean duration (ms) 514.3 531.9 +3.4%
    Median duration (ms) 511.1 533.9 +4.5%
    Mean CV 11.2% 13.4% +19.3%
    Worst duration (ms) 616.6 629.3 +2.1%
    Worst identifier React value

It looks like nothing changed too much. I’m pretty lenient since I’m still an experiment, so take a look anyways and make sure nothing looks out of place.

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

@typescript-bot typescript-bot moved this from Waiting for Reviewers to Check and Merge in Pull Request Status Board Jun 25, 2019
@typescript-bot typescript-bot added Owner Approved A listed owner of this package signed off on the pull request. Merge:Express and removed Awaiting reviewer feedback labels Jun 25, 2019
@typescript-bot
Copy link
Contributor

A definition owner has approved this PR ⭐️. A maintainer will merge this PR 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!

@uniqueiniquity uniqueiniquity merged commit bd40cfc into DefinitelyTyped:master Jun 26, 2019
Pull Request Status Board automation moved this from Check and Merge to Done Jun 26, 2019
@typescript-bot
Copy link
Contributor

I just published @types/braft-editor@1.9.3 to npm.

@typescript-bot
Copy link
Contributor

I just published @types/draft-convert@2.1.1 to npm.

@typescript-bot
Copy link
Contributor

I just published @types/draft-js@0.10.34 to npm.

iRON5 pushed a commit to iRON5/DefinitelyTyped that referenced this pull request Aug 13, 2019
@MunifTanjim
@types/draft-js fix mixed immutable version problem (DefinitelyTyped#…
7f19267 
…36402)

* @types/draft-js fix mixed immutable version problem

* bump typescript version for draft dependents
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@brunoMaurice brunoMaurice brunoMaurice approved these changes

Assignees
No one assigned
Labels
Owner Approved A listed owner of this package signed off on the pull request. Popular package This PR affects a popular package (as counted by NPM download counts).
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@MunifTanjim @typescript-bot @brunoMaurice @uniqueiniquity