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

Add type check to PropType on OneOf #2653

Merged
merged 8 commits into from
Feb 7, 2018
Merged

Add type check to PropType on OneOf #2653

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
ce6232b
Add type check to PropType on OneOf
theAbeGonzalez Jan 5, 2018
f44ff77
Add oneOf evaluation to examples
theAbeGonzalez Jan 5, 2018
0d86472
Update snapshots
theAbeGonzalez Jan 7, 2018
77c03d5
Merge branch 'master' into master
Hypnosphi Jan 8, 2018
13c18e0
Merge branch 'master' into master
ndelangen Jan 31, 2018
d65ea18
Merge remote-tracking branch 'origin/master' into abrahamgnz-master
Hypnosphi Feb 7, 2018
35d0740
Update storyshot
Hypnosphi Feb 7, 2018
149160c
Merge branch 'master' into master
Hypnosphi Feb 7, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion addons/info/src/components/types/OneOf.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,11 @@
import React from 'react';
import { TypeInfo } from './proptypes';

const OneOf = ({ propType }) => <span>{propType.value.map(({ value }) => value).join(' | ')}</span>;
const joinValues = propType => propType.value.map(({ value }) => value).join(' | ');

const OneOf = ({ propType }) => (
<span>{`oneOf ${Array.isArray(propType.value) ? joinValues(propType) : propType.value}`}</span>
);

OneOf.propTypes = {
propType: TypeInfo.isRequired,
Expand Down
1 change: 1 addition & 0 deletions examples/official-storybook/components/DocgenButton.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,7 @@ DocgenButton.propTypes = {
* `oneOf` is basically an Enum which is also supported but can be pretty big.
*/
enm: PropTypes.oneOf(['News', 'Photos']),
enmEval: PropTypes.oneOf((() => ['News', 'Photos'])()),
/**
* A multi-type prop is also valid and is displayed as `Union<String|Message>`
*/
Expand Down
25 changes: 25 additions & 0 deletions examples/official-storybook/stories/__snapshots__/addon-info.stories.storyshot
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3592,6 +3592,31 @@ exports[`Storyshots Addons|Info.React Docgen Comments from PropType declarations
class="css-d52hbj"
/>
</tr>
<tr>
<td
class="css-1ygfcef"
>
enmEval
</td>
<td
class="css-1ygfcef"
>
<span />
</td>
<td
class="css-d52hbj"
>
-
</td>
<td
class="css-d52hbj"
>
-
</td>
<td
class="css-d52hbj"
/>
</tr>
<tr>
<td
class="css-1ygfcef"
Expand Down
4 changes: 2 additions & 2 deletions examples/official-storybook/stories/__snapshots__/addon-knobs.stories.storyshot
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ exports[`Storyshots Addons|Knobs.withKnobs tweaks static values 1`] = `
My name is Storyteller, I'm 70 years old, and my favorite fruit is apple.
</p>
<p>
My birthday is: January 20, 2017
My birthday is: January 19, 2017
Copy link
Member

Choose a reason for hiding this comment

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

@tmeasday looks like your fix don't work in some cases

Copy link
Member

Choose a reason for hiding this comment

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

@abrahamgnz just for reference, which timezone are you in?

Copy link
Member

Choose a reason for hiding this comment

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

This is puzzling ;)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

CST @Hypnosphi

Copy link
Member

@tmeasday tmeasday Jan 8, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

@abrahamgnz it would be interesting to know what the following output in the node REPL for you:

new Date('Jan 20 2017 GMT+0').toLocaleDateString('en-US')

Copy link
Contributor Author

@theAbeGonzalez theAbeGonzalez Jan 8, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

+new Date('Jan 20 2017 GMT+0')
1484870400000

@tmeasday
I got the same

Copy link
Member

@Hypnosphi Hypnosphi Jan 8, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

@tmeasday looks like we have to add timeZone: 'UTC' to dateOptions
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Date/toLocaleDateString
en-US is a format not a timezone

the default is the runtime's default time zone

The midnight in London is definitely the same day in Netherlands, Russia, Israel, India, and Australia, that's why it worked for most of the core contributors =)

Copy link
Member

Choose a reason for hiding this comment

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

I can't believe no-one in the US ran into this yet... not even the CI servers!

I can't really explain why it was printing out the 19th in the first place for me then (given new Date('Jan 20 2017') was midnight 20 Jan in my TZ). But let's lock it down.

Copy link
Member

Choose a reason for hiding this comment

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

how?

Copy link
Member

Choose a reason for hiding this comment

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

This was probably fixed in #2861

@abrahamgnz can you please try to update snapshots once again?

</p>
<p>
I live in NY for 9 years.
Expand Down Expand Up @@ -73,7 +73,7 @@ exports[`Storyshots Addons|Knobs.withKnobsOptions tweaks static values with debo
My name is Storyteller, I'm 70 years old, and my favorite fruit is apple.
</p>
<p>
My birthday is: January 20, 2017
My birthday is: January 19, 2017
</p>
<p>
My wallet contains: $12.50
Expand Down