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

documentation for the why of "key" #82

Open
kswope opened this issue Jul 23, 2015 · 7 comments
Open

documentation for the why of "key" #82

kswope opened this issue Jul 23, 2015 · 7 comments

Comments

@kswope
Copy link

kswope commented Jul 23, 2015

I don't see a single use case anywhere for why keys even exist even though they are sprinkled throughout the docs, or how you would implement multiple keys.

Even in the very first example you find this line but no mention of "myKey" ever again.

var structure = immstruct('myKey', { a: { b: { c: 1 } } });

Some places even imply that keys are a ho-hum feature:

Provide optional key to be able to retrieve it from list of instances. If no key is provided, a random key will be generated.

optional: - defaults to random string

@mikaelbr
Copy link
Member

Hi! It's kind of documented in the README by this second example:

// anotherFile.js
var immstruct = require('immstruct');
var structure = immstruct('myKey');

var cursor = structure.cursor(['a', 'b', 'c']);

var updatedCursor = cursor.update(function (x) { // triggers `swap` in somefile.js
  return x + 1;
});

// Unwrap the value
console.log(updatedCursor.deref()); //=> 3

But I've updated that example to have a more explicit comment now.

@kswope
Copy link
Author

kswope commented Jul 23, 2015

Updated doc:

// anotherFile.js
var immstruct = require('immstruct');

// Get the structure we previously defined under the ID `myKey`
var structure = immstruct('myKey');

var cursor = structure.cursor(['a', 'b', 'c']);

var updatedCursor = cursor.update(function (x) { // triggers `swap` in somefile.js
  return x + 1;
});

// Unwrap the value
console.log(updatedCursor.deref()); //=> 3

The second line of code looks like its getting a fresh "immstruct thing" from the library, there is no indication of where myKey ever existed before! Oh is that section of code a continuation of the first one but in another file? Maybe it would be more clear with a somefile.js header above the first and anotherfile.js above the second.

I swear up until this point I had no idea requrie('immstruct') returned a singletonish instance! Thats probably my fundamental misunderstanding. I think that should be made more clear for the folks going isomorphic with it.

Way at the bottom of the page is this, which I always wondered about why it existed,now its obvious

var localImmstruct = new immstruct.Immstruct()

@mikaelbr
Copy link
Member

Yeah, it's a singleton kind of thing. Probably not the best initial design choice, as I'm not the biggest fan of singleton-instances, but it seemed reasonable and handy at the time.

At the top of the usage, the first example is marked with "someFile.js", but it might require "fine reading" as it doesn't stand out too much being a single line comment. I'll look into making this more clear! Thanks for your feedback.

Way at the bottom of the page is this, which I always wondered about why it existed,now its obvious

var localImmstruct = new immstruct.Immstruct();

Yeah, that's right. It's for the cases you want a isolated instance of immstruct, with your own internal list of structure instances. You can also have a isolated instance of Structure as well. So immstruct is a factory and "instance" manager for Structures, which is the actual implementation of immutable structures. You can also do:

var Structure = require('immstruct/structure');

var struct = new Structure({
  data: { foo: 1 }
});

if that's something you want.

@torgeir
Copy link
Member

torgeir commented Jul 23, 2015

Yeah, it's a singleton kind of thing. Probably not the best initial design choice, as I'm not the biggest fan of singleton-instanses, but it seemed reasonable and handy at the time.

Agreed, seems kind of weird. As long as you can create new instances, creating a singleton is a matter of creating a single module that does module.exports = instance.

@mikaelbr
Copy link
Member

Yeah, true, but on the other hand; it is convenient. But it stores state that is magically accessible through cached requires. If by some way you require a totally new instance of the module (would probably not happen), it would be two different singletons.

@kswope
Copy link
Author

kswope commented Jul 23, 2015

Is the most common use case of omniscient/immstruct to pass a single immstruct down through each component, and using that to pull out cursors, or to do a require('immstruct') at the top of each component file?

( I'm assuming that immstruct is most commonly used for omniscient. ).

Maybe the best practice, whatever that is, should get the most weight in the docs. At the moment it looks like the singleton approach is hugely preferred inferring from its popularity in the docs.

@mikaelbr
Copy link
Member

( I'm assuming that immstruct is most commonly used for omniscient. ).

There are those who use immstruct with vanilla React as well, or even without React/Omniscient as a whole.

Is the most common use case of omniscient/immstruct to pass a single immstruct down through each component, and using that to pull out cursors, or to do a require('immstruct') at the top of each component file?

The most common thing (or "best practice") is to pass the data through your components, as arguments to a function. But you could also look into component with references in those cases you actually need "horizontal" data injection. (There can be cases where it's impractical to pass down all data as input.

Maybe the best practice, whatever that is, should get the most weight in the docs. At the moment it looks like the singleton approach is hugely preferred inferring from its popularity in the docs.

You might have a good point here. Though, immstruct isn't necessarily a Omniscient.js library. All though it works great with it, you can easily use immstruct without Omniscient and visa versa.

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

No branches or pull requests

3 participants