stampit-org / stamp-specification

The Stamp Specification: Composables
434 stars 16 forks source link

Add support for ES6 Symbols #86

Closed koresar closed 8 years ago

koresar commented 8 years ago

The specs doesn't say a thing about ES6 Symbols. We should support Symbols by default.

Here is the feature request from @FredyC: https://github.com/stampit-org/supermixer/issues/6

danielkcz commented 8 years ago

Symbols are generally great thing to hide something that should not be revealed too easily. Would be great to have built in support for private instance variables and methods with symbols, but I am unsure how that would look like.

Either way having ability to use Symbol as name of method or property is definitely first step.

Here is how I've used this with Symbols, perhaps it will strike some inspiration here.

implementation of instanceOf alternative making property value truly private

danielkcz commented 8 years ago

I have read carefully through #84 which is mostly about protected/shared values. However nobody in there actually talks about private values shared within stamp instance methods. Note: It's very likely that my mindset is still in OOP and I am just missing something the stamps are offering.

I've seen all over place creating closure inside initializer and exposing value through public method. That's fine I suppose, but what if I don't want to expose it? I simply would like to have some place where I can keep data that only that instance can access. Here is my use case:

function initializeTreeModel({ id: treeId }, { stamp }) {
    const { createNode } = stamp.compose.configuration;

    stamp.compose.methods.addNode = (nodeName, nodeProperties) => {
        const nodeInstance = createNode(nodeName, nodeProperties);
                this.nodes.add(nodeInstance);
        return nodeInstance;
    };
}

What I don't like here is that addNode function is defined for each instance separately, totally unnecessary object waste in my opinion. I don't want to expose the createNode because it would be there only to confuse user since the node output from createNode would not be useful on outside. So here goes solution with the Symbol...

const bCreateNode = Symbol('function to create a new node');

export default Model('Tree')
    .init(initializeTreeModel)
    .methods({ addNode })
;

function initializeTreeModel({ id: treeId }, { stamp }) {
        this[bCreateNode] = stamp.compose.configuration.createNode
}

function addNode(nodeName, nodeProperties) {
    const nodeInstance = this[bCreateNode](nodeName, nodeProperties);
    this.nodes.add(nodeInstance);
    return nodeInstance;
};

Isn't that so much more easier to read and comprehend? No magic is happening anywhere, you see plain code of functions accessing clearly defined variable.

What could be other ways to have a truly private state within stamp instance? Is there any? I can essentially only think of some transpiler plugin that do the magic. Question is why to go through hard way when there is such straightforward one?

Also I don't buy argument that Symbol it's not implemented in all enviroments yet, it's (actually just IE11 and lower) and I am willing to supply suboptimal polyfills for this group of browsers.

koresar commented 8 years ago

Note: It's very likely that my mindset is still in OOP and I am just missing something the stamps are offering.

Looks like this is the case. I'm pretty sure you are looking for "shared" state as we call it. I.e. data shared between different stamps.

What could be other ways to have a truly private state within stamp instance? Is there any? I can essentially only think of some transpiler plugin that do the magic. Question is why to go through hard way when there is such straightforward one?

Your "Symbol" solution does exactly that - the shared state. Also, symbols are not really private, they are public. One just need to use a different JS function to access symbols - Object.getOwnPropertySymbols.

The only way to have truly private data in JavaScript are closures.

What I don't like here is that addNode function is defined for each instance separately, totally unnecessary object waste in my opinion.

You have just did a performance micro optimization. "Premature optimization is the root of all evil." :) However, you can put if clause to optimize that bit:

if (!stamp.compose.methods.addNode)
    stamp.compose.methods.addNode = (nodeName, nodeProperties) => {
        const nodeInstance = createNode(nodeName, nodeProperties);
        this.nodes.add(nodeInstance);
        return nodeInstance;
    };

Also I don't buy argument that Symbol it's not implemented in all enviroments yet.

Me neither! It's just makes it harder to implement, but it's totally doable.

Isn't that so much more easier to read and comprehend?

I truly believe stamps should support Symbols. Can you draft the changes to the specification as you see them?

danielkcz commented 8 years ago

Looks like this is the case. I'm pretty sure you are looking for "shared" state as we call it. I.e. data shared between different stamps.

Except I don't want to share data between stamps, but within single stamp instance. I believe that's whole different category...

Anyway I kinda ditched Symbol for this now as I have found this blog post by accident. It's not that nice and easy to use in overall, but it has given me truly private data that can be still accessed by stamp methods. Eg. I can do this as far as I have reference to instance: privates.set(this, 'nodes', new Set());

On the topic now. I am not sure much about specification itself, should be simply understood that Symbol is essentially same as any other property except it's not just a string. Any Symbol property that appears on any part of descriptor is treated as regular value property.

As for implementation, first step is surely to use own assign/merge utilities because lodash doesn't seems it will go that way for some time. I did not found any true confirmation, but no point waiting for that I think.

koresar commented 8 years ago

My apologies. We confuse terminology. When I say "data shared between different stamps" I really mean "an instance data shared between different stamp implementations". Surely, I did not mean "sharing data between Stamps themselves".

Thank you for the WeakMaps link. This one is very helpful. I would be recommending the blog post to others in the future.

AFAIK lodash is not implementing Symbols support, at least for now. See https://github.com/lodash/lodash/issues/2088

koresar commented 8 years ago

In regards of implementing Symbols as part of the specification.

IMO we should wait until lodash supports Symbols. Not because it's too much effort, but because it would simply mean that browsers (and the world) are ready for it.

danielkcz commented 8 years ago

IMO we should wait until lodash supports Symbols. Not because it's too much effort, but because it would simply mean that browsers (and the world) are ready for it.

I don't simply agree with that. How do you want to be more ready? If you look at the current support all browsers are at 100% except IE11 which isn't going to implement that anyway, M$ is not touching that thing regarding ES6.

koresar commented 8 years ago

Okay. I'll see what I can do. (Meanwhile, feel free to submit a PR.)

koresar commented 8 years ago

Looks like Symbols are not fully implemented:

Even node.js v6 have only half of Well Known Symbols. Safari, Chrome, FF are not really there too.

danielkcz commented 8 years ago

Um, why do we need to care about well known symbols? We are only interested here in Symbol as non-string property, nothing more. You need well know symbols if you are going to actually do something with them which stamp isn't.

Edit: With Safari it's essentially the same story as with IE. They don't have any release cycle to accommodate for fast changes that are happening.

koresar commented 8 years ago

@unstoppablecarl @ericelliott @troutowicz Do you guys think the specification should mention ES6 Symbols? Should stamps support Symbols in general?