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

types: add Symbol #134

Merged
merged 1 commit into from
Apr 14, 2017
Merged

Conversation

gabejohnson
Copy link
Member

Closes #133

@gabejohnson
Copy link
Member Author

@davidchambers will this change require adding Symbol to sanctuary-type-classes as well?

@davidchambers
Copy link
Member

@davidchambers will this change require adding Symbol to sanctuary-type-classes as well?

I don't think so. What do you have in mind?

index.js Outdated
@@ -713,6 +713,12 @@
//. Type comprising every primitive String value.
var String_ = NullaryTypeWithUrl('String', typeofEq('string'));


Copy link
Member

Choose a reason for hiding this comment

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

Nit: Unwanted line.

index.js Outdated

//# Symbol :: Type
//.
//. Type comprising every primitive Symbol value.
Copy link
Member

Choose a reason for hiding this comment

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

Is it necessary to use "primitive" here?

Copy link
Member Author

Choose a reason for hiding this comment

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

An artifact of copying. There is no boxed version of Symbol thus no need to differentiate it.

Though it is a primitive.

index.js Outdated
//# Symbol :: Type
//.
//. Type comprising every primitive Symbol value.
var Symbol_ = NullaryTypeWithUrl('Symbol', typeofEq('symbol'));
Copy link
Member

Choose a reason for hiding this comment

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

Is there a reason to favour typeofEq('symbol') over typeEq('Symbol'), or are the expressions equivalent?

Copy link
Member Author

Choose a reason for hiding this comment

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

They should be equivalent. I would think the typeofEq would be slightly faster. I don't know if there are any benchmarks though.

Copy link
Member

Choose a reason for hiding this comment

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

👍

index.js Outdated
@@ -793,6 +800,7 @@
RegExp_,
StrMap(Unknown),
String_,
Symbol,
Copy link
Member

Choose a reason for hiding this comment

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

s/Symbol/Symbol_/

@gabejohnson
Copy link
Member Author

Updated

@davidchambers davidchambers merged commit 2189b6f into sanctuary-js:master Apr 14, 2017
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.

2 participants