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

Support for Enum Types #18

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Conversation

udnisap
Copy link
Contributor

@udnisap udnisap commented Feb 18, 2016

No description provided.

@@ -1,7 +1,7 @@
import test from "./test"
import * as Immutable from "immutable"
import {Record} from "../record"
import {Typed, typeOf, Union, Range, Maybe} from "../typed"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this was not used in the tests

@Gozala
Copy link
Collaborator

Gozala commented Jun 23, 2016

Unfortunately I am no longer able to maintain this project, I apologize. If someone wishes to step up and take leadership of this project please respond to #21

@udnisap udnisap mentioned this pull request Jul 4, 2016


return Typed(`Enum(${validValues})`, value => {
if (!map[value] && value !== undefined){
Copy link
Member

Choose a reason for hiding this comment

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

I don't think the value !== undefined check should be there - if user want's an optional value shouldn't they choose to use Maybe?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good call. will change that.

@lukesneeringer
Copy link
Contributor

Do we want this in the main library? It's trivial to just make a Typed that does it.
I have the same thing in my codebase, except I spelled it Choice.

That said, probably this is a pretty common use case, and deserves inclusion.

@davecoates
Copy link
Member

Seems common enough that it's probably worth including - I've added something equivalent on every project I've used typed-immutable on.

@stutrek
Copy link
Member

stutrek commented Jul 19, 2016

I like this too, seems like a consensus. @udnisap can you change the value !== undefined? I would like to release a new minor version this week.

@lukesneeringer
Copy link
Contributor

@udnisap Can you merge in master and push, so that Travis can pick it up and test this PR?

@davecoates
Copy link
Member

Sorry, we need docs for this as well

@lukesneeringer lukesneeringer mentioned this pull request Jul 27, 2016
@udnisap
Copy link
Contributor Author

udnisap commented Jul 28, 2016

@stutrek had to refactor the tests and also to add the first value as the default value.

@stutrek
Copy link
Member

stutrek commented Jul 28, 2016

Great @udnisap! We will need to add this to the readme. Let me know if you need any help with that.

@lukesneeringer
Copy link
Contributor

@udnisap I am ready to cut a release as soon as this is readme-d and ready.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants