Skip to content
This repository was archived by the owner on Jun 8, 2019. It is now read-only.

allow object passed to description prop#87

Merged
ericf merged 4 commits into
formatjs:masterfrom
mattkime:more_metadata_v2
Dec 29, 2016
Merged

allow object passed to description prop#87
ericf merged 4 commits into
formatjs:masterfrom
mattkime:more_metadata_v2

Conversation

@mattkime
Copy link
Copy Markdown
Contributor

Follow up to #85 (comment)

Allows objects to be passed to the description prop.

Only code change required was to check the description type before .trim()

Added test to verify this works.

@mattkime mattkime changed the title More metadata v2 allow object passed to description prop Dec 27, 2016
Copy link
Copy Markdown
Collaborator

@ericf ericf left a comment

Choose a reason for hiding this comment

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

@mattkime this is great, I'm glad it was such a simple change! Thanks for adding the test too. I added a few minor comments about formatting stuff. If you don't have a change to make these changes, let me know and I can just make them.

Comment thread scripts/build-fixtures.js Outdated
['moduleSourceName', {
moduleSourceName: 'react-i18n',
}],
'descriptionsAsObjects'
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Can you move this right after defineMessages so the fixtures remain in alpha order?

Comment thread src/index.js Outdated

// Always trim the Message Descriptor values.
return evaluatePath(path).trim();
let descriptorValue = evaluatePath(path);
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

const

Comment thread src/index.js Outdated
return evaluatePath(path).trim();
let descriptorValue = evaluatePath(path);

if( typeof descriptorValue === 'string' ){
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Update formatting to match rest of file:

if (condition) {

Comment thread src/index.js Outdated
let descriptorValue = evaluatePath(path);

if( typeof descriptorValue === 'string' ){
descriptorValue = descriptorValue.trim();
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This can be an early return:

if (typeof descriptorValue === 'string') {
    return descriptorValue.trim();
}

@ericf ericf merged commit 429956c into formatjs:master Dec 29, 2016
This was referenced Dec 29, 2016
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.

2 participants