So #92 was somewhat resolved with this PR #120
The next step is to eventually publish a version of fantasy-land npm package with namespaced prefixed names.
There are some questions to discuss though:
Maybe there more questions, these two is what came to my mind.
I've faced a couple of problems with the new approach. Not sure where better to document them, hopefully this issue is fine.
of is a reserved word, we can't use it for a variable name.diff
- import {map, of} from 'fantasy-land'
+ import {map, of as just} from 'fantasy-land'
diff
- const {map, of} = require('fantasy-land')
+ const {map, of: just} = require('fantasy-land')
js
// Alternatively just import whole object to a variable
import fl from 'fantasy-land'
foo[fl.of](1)
Maybe we could add an alias for of in index.js (e.g. just)
class syntax nor via .prototype):
src/fantasy.js:34
34: [concat]( other:Stream<T> ): Stream<T> {
^ computed property keys not supported
src/fantasy.js:147
147: Stream.prototype[concat] = Stream.prototype.concat
^^^^^^^^^^^^^^^^^^^^^^^^ assignment of computed property/element. Indexable signature not found in
147: Stream.prototype[concat] = Stream.prototype.concat
^^^^^^ Stream
A workaround I found so far looks like this:
``` js
addFLmethods(Stream.prototype)
function addFLmethods( proto:any ): void {
proto[concat] = proto.concat
}
```
But this is only a workaround for adding methods to a class, in any case we bypass Flow type checking when will use those class instances. And in general dynamic names can be a problem for any static analysis tools like Flow/TypeScript/etc.
I was thinking, maybe in the future we could start treating all this as a temporal measure for the transition period and go back to using hardcoded names but with prefixes.
Another issue: how to use FL compatible libraries in the browser (e.g. in jsfiddle)?
Options I see so far (here foo is a FL compatible lib):
fantasy-land package UMD compatible. Consumers then could load in browser two UMD libs (FL and foo) and they should work fine together (probably, I'm not really sure how dependencies work in UMD).fantasy-land as part of the browser build of foo. This is not a great idea, but still could work for things like jsfiddle as long as all libs that are used bundled with the same version of FL.foo (basically same as 2. with backward compatibility version of FL).hardcoded names but with prefixes
I'm still not convinced by this, because I don't use ramda or alternatives, so if we do prefix things it just means it's more of a pain for me and an extra barrier for using fantasy-land, the great thing about the spec is that it can be used in both!
Functor.prototype['fl/map'] = function(f) {};
// As an example.
const f = ...
f['fl/map'](x => x);
I wonder if there is a way to have both without sacrificing anything?
of is a reserved word, we can't use it for a variable name
But this is a tooling/linting issue not actual code issue, because running this doesn't cause any issues. So either we create an alias to solve the tooling/linting issue or we just change the name completely. I'm not a fan of just, I'd rather have pure.
Also @rpominov thanks for the feedback, we should consider all of the above going forward.
:+1:
@rpominov of is _not_ a reserved word. Did some tool fail or did you just make that assumption?
@SimonRichardson @michaelficarra Yeah, you are right about of. Seems like only Flow has problems with it:
src/fantasy.js:4
4: import {of} from 'fantasy-land'
^^ Unexpected token of
I'll open an issue in Flow repo...
Thanks @rpominov. Please link it here when you do. I'm very interested.
:+1:
One thing, you're right that Flow doesn't currently support computed keys in a number of scenarios, and can't in general, but in the specific cases where the string literal information can be known statically it's possible. We already support string literal types, which enable this feature. I'll take a look to see if we can easily support this use case.
@samwgoldman That would be great! So yeah computed names are just strings that come from this file index.js I think it could be possible to statically analyse, at least theoretically :)
I wonder if there is a way to have both without sacrificing anything? -- @SimonRichardson
In Fluture I did this thing where I create keys for both 'map' and FL.map on my type, so when FL adds namespaces, my API won't change, and FL dispatchers will still work.
I think this is the best way to do it, create normal methods in your API like with "old" FL, and then do something like:
Type.prototype[FL.map] = Type.prototype.map
Type.prototype[FL.ap] = Type.prototype.ap
...
We could even provide a helper that does this automatically:
import {makeFantasyLandCompatible} from 'fantasy-land/compat-helper'
makeFantasyLandCompatible(Type)
Just a curious question: why isn't Symbol.for being used? I feel something like exporting Symbol.for("fantasy-land/ap") may work fairly well, since Symbol.for is required to work cross-realm per ES2015+ spec.
Symbols may cause some pain in legacy JS environments. Also Flow doesn't currently support Symbols for instance.
On the other hand it's not clear how Symbol.for("fantasy-land/ap") better than just "fantasy-land/ap" for our use-case.
Edit: Although Flow doesn't support dynamic method names either. But anyway Symbols may cause problems and don't give anything in return as it seems.
More discussion on the subject of Symbol.for('fantasy-land/x') can be found here
@rpominov I came up with a possible solution to that here.
But I'm still trying to see why namespacing is even necessary at all. Reading the previous discussion on namespacing the methods hasn't really convinced me.
This is done.
@samwgoldman We now have switched to constant prefixed names, which Flow can support but unfortunately doesn't yet.
Created an issue in Flow repo https://github.com/facebook/flow/issues/2482
Most helpful comment
I think this is the best way to do it, create normal methods in your API like with "old" FL, and then do something like:
We could even provide a helper that does this automatically: