Skip to content

Conversation

@cshaa
Copy link
Contributor

@cshaa cshaa commented Feb 23, 2019

fixes #29307, possibly a breaking change

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).

If changing an existing definition:

  • Provide a URL to documentation or source code which provides context for the suggested changes: Babel link. It's clear from here that fragments are just React Elements of the React.Fragment component.
  • 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 Popular package This PR affects a popular package (as counted by NPM download counts). Awaiting reviewer feedback labels Feb 23, 2019
@typescript-bot
Copy link
Contributor

typescript-bot commented Feb 23, 2019

@m93a Thank you for submitting this PR!

🔔 @johnnyreilly @bbenezech @pzavolinsky @digiguru @ericanderson @tkrotoff @DovydasNavickas @onigoetz @theruther4d @guilhermehubner @ferdaber @jrakotoharisoa @pascaloliv @Hotell @franklixuefei @Jessidhia @pshrmn @threepointone - 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.

@ferdaber
Copy link
Contributor

@Jessidhia I forgot, what was the blocker for removing this other than the high amount of breakage this causes?

@cshaa
Copy link
Contributor Author

cshaa commented Feb 23, 2019

Oh, it's probably failing because many packages pass functions or custom objects as children. This shouldn't have anything to do with Fragments, however. Maybe adding | object to the ReactNode or ReactChildren would be a good idea?

I'm certain that non-primitive object type is much better than {} (which is almost any).

@typescript-bot
Copy link
Contributor

typescript-bot commented Feb 24, 2019

@m93a 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!

@cshaa
Copy link
Contributor Author

cshaa commented Feb 24, 2019

I've looked at the errors in Travis. Most of them seem unrelated to the PR 🤔. Could you please help me out with this?

@ferdaber
Copy link
Contributor

A lot of those are from untouched dependencies that haven't been retested since newer versions of TS, but the recompose ones look interesting, did you take a look at those?

@typescript-bot
Copy link
Contributor

@m93a I haven't seen anything from you in a while and this PR currently has problems that prevent it from being merged. The PR will be closed tomorrow if there aren't new commits to fix the issues.

@typescript-bot typescript-bot added the Abandoned This PR had no activity for a long time, and is considered abandoned label Mar 2, 2019
@typescript-bot
Copy link
Contributor

@m93a To keep things tidy, we have to close PRs that aren't mergeable but don't have activity from their author. No worries, though - please open a new PR if you'd like to continue with this change. Thank you!

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

Labels

Abandoned This PR had no activity for a long time, and is considered abandoned Popular package This PR affects a popular package (as counted by NPM download counts).

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Type of React.ReactFragment, which includes {}, breaks certain usages of children in TS 3.1

3 participants