Skip to content

Conversation

@hirokith
Copy link
Contributor

@hirokith hirokith commented Jul 5, 2016

No description provided.

@hirokith
Copy link
Contributor Author

Two weeks gone, is there anyone maintaining this package's upgrade? Need this pr to be merged badly!

@fkling
Copy link
Member

fkling commented Jul 18, 2016

Sorry, I've been on vacation for exactly two weeks :)

returns: null,
params: [{
name: 'bar',
type: {name: 'string|Object'},
Copy link
Member

Choose a reason for hiding this comment

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

I'm wondering if this shouldn't rather be something like {name: 'union', value: ['string', 'Object']}. What do you think @janicduplessis?

Copy link
Contributor

@janicduplessis janicduplessis Jul 18, 2016

Choose a reason for hiding this comment

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

I agree we should keep this as an object since it's easier to use after. We should try to have a similar output object as the flow type definitions.

/**
 * @param test Test
 */
function (test: string | Object) {}

and

/**
 * @param {string | Object} test Test
 */
function (test) {}

should result in the same thing.

You could take a look at what the object we return for flow types look like and return something similar.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK, I'll update the commit soon.

@hirokith
Copy link
Contributor Author

Another week gone, so when will this to be merged and when will next version to be published?

@fkling
Copy link
Member

fkling commented Aug 2, 2016

Sorry for the slow progress here, the last couple of weeks just had been really busy (I was traveling most of the time).

If we want to stay compatible with flow types, I think * should be mapped to mixed or any.

@hirokith
Copy link
Contributor Author

hirokith commented Aug 3, 2016

OK, thank you for your suggestion, I've updated the commit~

@danez
Copy link
Collaborator

danez commented Aug 11, 2016

👍

@fkling fkling merged commit 0edacd7 into reactjs:master Aug 22, 2016
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 29, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants