-
Notifications
You must be signed in to change notification settings - Fork 0
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
Use a thunk instead of a direct reference to the canonical module #2
Conversation
specification.md
Outdated
@@ -90,15 +90,15 @@ All methods' implementations should only use type information about arguments th | |||
|
|||
## Canonical Module | |||
|
|||
A value may have a reference to a canonical module that works with values of that value's type. The reference should be in the `fantasy-land/canonical` property. For example: | |||
A value may have a reference to a canonical module that works with values of that value's type. Such value should have a method named `fantasy-land/canonical` that returns the module. For example: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Such a value would read better than Such value.
specification.md
Outdated
``` | ||
|
||
Note that the `ListModule2` here is correct. Only the `list` value doesn't follow the specification. | ||
|
||
The `fantasy-land/canonical` method must always return equivalent modules when called more than one times. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suggest replacing more than one times with multiple times.
85082b8
to
7e449a0
Compare
@davidchambers Thanks! Updated. |
}, | ||
map(f, v) { | ||
return {'fantasy-land/canonical': ListModule, data: v.data.map(f)} | ||
return {'fantasy-land/canonical': () => ListModule, data: v.data.map(f)} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think f and v will not be gc-ed as fantasy-land/canonical has reference to it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As far as I know, compilers can notice that these variables are not used in the function so they won't count this as references during garbage collection. But I'm not an expert.
Also in real code I wouldn't create the function on a spot like this, but would create it once and then reuse. Not sure though if we should do this in the example and complicate it.
``` | ||
|
||
Note that the `ListModule2` here is correct. Only the `list` value doesn't follow the specification. | ||
|
||
The `fantasy-land/canonical` method must always return equivalent modules when called multiple times. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't quite understand what's the point of fantasy-land/canonical
being a function. like if we ask it always return equivalent modules when called multiple times, i.e. being pure, then it's only advantage I see is being able to lazily contract the value, which i don't see as significant optimization, I also haven't read that conversation so all this might be discussed in that issue
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the discussion boils down to this sentence:
By not making it a thunk the API is imposing an evaluation strategy, which in JavaScript in particular is a pretty serious problem due to the order of evaluation. (c) @alexandru
I'm also not sure about the importance of such flexibility, but on the other hand seems like it won't hurt anyway.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@alexandru can you point out "practical" example where fantasy-land/canonical being lazy is more useful/better then it being non lazy?
Using thunks would cause an unnecessary overhead for all implementors that have a static constant Maybe I'm missing something, but wouldn't you get the same thing simply by implementing const myThing = {
...
get [fantasy-land/canonical]() {
// do stuff
return { ... }
}
}; |
Good point about getters! I think they are even closer to Haskell's thunks than functions. And they should solve later binding problem from the fantasyland/static-land#45 (comment) . Classes also support them e.g. @alexandru what do you think, can we close this? |
This was proposed by @alexandru in fantasyland/static-land#45 (comment) . It should make specification more flexible.
I don't have a strong opinion on this, and if there are no objections this seems like a good idea.