unionOf -- extend polymorphic unions to fields #42

Merged
merged 3 commits into from Jan 13, 2016

Conversation

3 participants
@eyston
Contributor

eyston commented Dec 11, 2015

For collection fields arrayOf and valuesOf both allow polymorphic schemas if you specify a schemaAttribute option. I didn't see the ability to do this for a non-collection field. The use case for this is if a field can also be polymorphic.

Mentioned in issue : #40

An example would be a group where the owner could be a group or a user.

    var user = new Schema('users'),
        group = new Schema('groups'),
        owner = unionOf({
          users: user,
          groups: group
        }, { schemaAttribute: 'type' });

    group.define({
      owner: owner,
    });

This pull request adds the syntax unionOf. It mimics the behavior found in IterableSchema but applies it in a non-iterable manner.

This could also pull the polymorphic logic out of IterableSchema by making itemSchema an instance of UnionSchema (the added type) if schemaAttribute is specified.

arrayOf(unionOf({ groups: group, users: user}, { schemaAttribute: 'type' })) === arrayOf({ groups: group, users: user }, { schemaAttribute: 'type' })

This is my fist pull request so let me know if I'm doing this wrong! Thanks :)

@eyston

This comment has been minimized.

Show comment
Hide comment
@eyston

eyston Dec 11, 2015

Contributor

the commit which removes polymorphic functionality from IterableSchema / visitIterable looks like:

eyston@99ff5c4

can merge that into this PR too if desired

Contributor

eyston commented Dec 11, 2015

the commit which removes polymorphic functionality from IterableSchema / visitIterable looks like:

eyston@99ff5c4

can merge that into this PR too if desired

@gaearon

This comment has been minimized.

Show comment
Hide comment
@gaearon

gaearon Jan 1, 2016

Collaborator

I'm not currently actively using this module so it's a bit difficult for me to review.
@unindented What do you think?

Collaborator

gaearon commented Jan 1, 2016

I'm not currently actively using this module so it's a bit difficult for me to review.
@unindented What do you think?

@gaearon

This comment has been minimized.

Show comment
Hide comment
@gaearon

gaearon Jan 1, 2016

Collaborator

This looks good to me on a glance. I'm happy to have eyston/normalizr@99ff5c4 here too. Mind merging it here?

Collaborator

gaearon commented Jan 1, 2016

This looks good to me on a glance. I'm happy to have eyston/normalizr@99ff5c4 here too. Mind merging it here?

@eyston

This comment has been minimized.

Show comment
Hide comment
@eyston

eyston Jan 1, 2016

Contributor
Contributor

eyston commented Jan 1, 2016

@gaearon

This comment has been minimized.

Show comment
Hide comment
@gaearon

gaearon Jan 1, 2016

Collaborator

Can you please add it to the API docs in README and write release notes for the version bump?

Collaborator

gaearon commented Jan 1, 2016

Can you please add it to the API docs in README and write release notes for the version bump?

@eyston

This comment has been minimized.

Show comment
Hide comment
@eyston

eyston Jan 1, 2016

Contributor

Yup, I didn't want to write docs until after any feedback ;p

Contributor

eyston commented Jan 1, 2016

Yup, I didn't want to write docs until after any feedback ;p

@unindented

This comment has been minimized.

Show comment
Hide comment
@unindented

unindented Jan 3, 2016

Collaborator

Looks good to me.

Collaborator

unindented commented Jan 3, 2016

Looks good to me.

@gaearon

This comment has been minimized.

Show comment
Hide comment
@gaearon

gaearon Jan 6, 2016

Collaborator

Then I'm ready to publish after docs are added!

Collaborator

gaearon commented Jan 6, 2016

Then I'm ready to publish after docs are added!

@eyston

This comment has been minimized.

Show comment
Hide comment
@eyston

eyston Jan 7, 2016

Contributor

Thanks for your patience, I spent the last several days in a rabbit hole so put this off ;p.

I've added docs to the README but I'm not sure where to put the release notes.

Contributor

eyston commented Jan 7, 2016

Thanks for your patience, I spent the last several days in a rabbit hole so put this off ;p.

I've added docs to the README but I'm not sure where to put the release notes.

@gaearon

This comment has been minimized.

Show comment
Hide comment
@gaearon

gaearon Jan 8, 2016

Collaborator

Just post them here as a comment and I'll copy and paste.

Collaborator

gaearon commented Jan 8, 2016

Just post them here as a comment and I'll copy and paste.

@eyston

This comment has been minimized.

Show comment
Hide comment
@eyston

eyston Jan 8, 2016

Contributor

I found https://github.com/gaearon/normalizr/releases ;p

Here is my attempt!

  • Adds support for polymorphic fields via unionOf(schemaMap, options) which works like the polymorphic support for arrayOf and valuesOf but for non-collection fields (#40).
Contributor

eyston commented Jan 8, 2016

I found https://github.com/gaearon/normalizr/releases ;p

Here is my attempt!

  • Adds support for polymorphic fields via unionOf(schemaMap, options) which works like the polymorphic support for arrayOf and valuesOf but for non-collection fields (#40).

gaearon added a commit that referenced this pull request Jan 13, 2016

Merge pull request #42 from eyston/unionof
unionOf -- extend polymorphic unions to fields

@gaearon gaearon merged commit 5ede07e into paularmstrong:master Jan 13, 2016

1 check passed

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

This comment has been minimized.

Show comment
Hide comment
@gaearon

gaearon Jan 13, 2016

Collaborator

Out in 2.0.0!

Collaborator

gaearon commented Jan 13, 2016

Out in 2.0.0!

@lock

This comment has been minimized.

Show comment
Hide comment
@lock

lock bot May 7, 2018

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

lock bot commented May 7, 2018

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot added the Outdated label May 7, 2018

Repository owner locked as resolved and limited conversation to collaborators May 7, 2018

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