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.

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

feat: Exporting values. (#396) #398

Merged
merged 8 commits into from Feb 8, 2018

Conversation

Projects
None yet
2 participants
@AndrewLeedham
Copy link
Contributor

commented Feb 8, 2018

Added defined values to the exported JSON object. As discussed in #396

AndrewLeedham added some commits Feb 7, 2018

feat: Exporting values. (#396)
Added defined values to the exported JSON object.
file.exports = message(result, "classes");
file.exports = Object.assign(
Object.create(null),
mapValues(file.values, (obj) => [ obj.value ]),

This comment has been minimized.

Copy link
@tivac

tivac Feb 8, 2018

Owner

Since @value statements don't have a composition graph to follow I don't think wrapping them in an array is worth it. This can probably mix in file.values directly instead.

Side benefit is that classes & @values are easily differentiated just by glancing at them.

This comment has been minimized.

Copy link
@tivac

tivac Feb 8, 2018

Owner

Whoops, ignore that. Forgot that the output method expects everything to be an array so it can .join them. This still feels a bit weird, but it's easier to go w/ the flow on it than to add special logic elsewhere.

This comment has been minimized.

Copy link
@AndrewLeedham

AndrewLeedham Feb 8, 2018

Author Contributor

Yeah, I played with changing the join function to check if it is an array. But it made more sense just to wrap the value in an array.

@tivac

This comment has been minimized.

Copy link
Owner

commented Feb 8, 2018

Good sleuthing! I forgot about the values-export plugin that merges that all together.

@tivac

tivac approved these changes Feb 8, 2018

@tivac

This comment has been minimized.

Copy link
Owner

commented Feb 8, 2018

Would you run npm test -- -u to update all the snapshot tests w/ the new output? Travis should succeed after that.

AndrewLeedham added some commits Feb 8, 2018

@tivac tivac merged commit 479866d into tivac:master Feb 8, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@tivac

This comment has been minimized.

Copy link
Owner

commented Feb 8, 2018

🎉

tivac added a commit that referenced this pull request Feb 9, 2018

feat: Exporting values. (#396) (#398)
BREAKING CHANGE: Values will now be exported alongside composed classes

Fixes #396
@tivac

This comment has been minimized.

Copy link
Owner

commented Feb 9, 2018

This is published as 8.0.0, thanks again!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.