These are chat archives for ericelliott/stampit

24th
Apr 2015
Eric Elliott
@ericelliott
Apr 24 2015 01:16
@korsesar - Don't let me block progress. You're a contributor with commit access. Please feel free to organize, coordinate, and get the changes in. When there's a release ready to cut, I'll update npm.
@koresar Please consider yourself the active maintainer for the time being. You have the keys to the kingdom. Feel free to use them.
Vasyl Boroviak
@koresar
Apr 24 2015 01:19
Ok. I'll merge the PR to branch 2.0
Eric Elliott
@ericelliott
Apr 24 2015 01:19
I merged it to 2.0 already.
Let me know when you're ready to cut an official 2.0 release and I'll update npm.
Vasyl Boroviak
@koresar
Apr 24 2015 01:21
Not ready. People are still needed the copying by ref feature.
Eric Elliott
@ericelliott
Apr 24 2015 01:21
We should add travis-ci and run local tests prior to merges
Vasyl Boroviak
@koresar
Apr 24 2015 01:21
+1
Would you submit an issue?
I believe something like this is necessary:
var StampWhichHoldDbConnectionReference = stampit().state({ ... }).refs({ dbConnection: whatever });
Eric Elliott
@ericelliott
Apr 24 2015 01:25
hmm
Vasyl Boroviak
@koresar
Apr 24 2015 01:26
Moreover, I would change the stamp-factory first argument from state to refs.
var myObject = MyStamp.create({ ref1: value1 });
Eric Elliott
@ericelliott
Apr 24 2015 01:26
wow
Vasyl Boroviak
@koresar
Apr 24 2015 01:27
yeah
Eric Elliott
@ericelliott
Apr 24 2015 01:27
refs is the new state? ;)
Vasyl Boroviak
@koresar
Apr 24 2015 01:27
nope, that would be an addition to fixed. Like MyStamp.fixed.refs.
Eric Elliott
@ericelliott
Apr 24 2015 01:28
yeah, I got that, but refs would basically have the same effect as .state() in 1.x?
Vasyl Boroviak
@koresar
Apr 24 2015 01:28
People relly need this. Too many of them are asking the same question(s).
Exactly.
var myObject = MyStamp.create({ ref1: value1 }); <- this would be stampit 1.x compatible.
Eric Elliott
@ericelliott
Apr 24 2015 01:29
any great reason not to leave .state() the way it is now (for backward compat reasons), and make a new method for the deep clone?
or maybe we could leave .state() alone and add .props() which could be the deep-cloned version?
Vasyl Boroviak
@koresar
Apr 24 2015 01:31
It depends on what people think of state usually. Is it "per instance safe state" or "per factory state" ?
hm...
As for me props is the synonym to refs.
Whereas, state is something very safe, deep cloned thing. IMHO
Eric Elliott
@ericelliott
Apr 24 2015 01:33
we could deprecate .state() entirely and use .refs() and .someOtherThing()
why does .props() sound like a synonym to .refs()? Could .props() be the deep cloned version, instead?
Vasyl Boroviak
@koresar
Apr 24 2015 01:36
There are utility-belt methods (in lodash for example) _.property. They operate references.
Eric Elliott
@ericelliott
Apr 24 2015 01:37
Well, property is the generic word for any member identifier in the language proper. JS as a whole doesn't have an opinion about it.
For instance, React props can go either way, if I recall correctly.
Vasyl Boroviak
@koresar
Apr 24 2015 01:39
I bet if we do one more poll I'll win again. :)
Eric Elliott
@ericelliott
Apr 24 2015 01:39
What name would you prefer?
Vasyl Boroviak
@koresar
Apr 24 2015 01:40
state - for deep cloned things. refs or props for shallow.
Eric Elliott
@ericelliott
Apr 24 2015 01:40
I'm not hooked on .props() -- just don't want to change what .state() means, because we're going to break existing code and force people to migrate their code to use 2.0...
Vasyl Boroviak
@koresar
Apr 24 2015 01:41
true
I'd rather make 2.0 release as compatible as possible.
Eric Elliott
@ericelliott
Apr 24 2015 01:41
Keep the open/closed principle in mind... some breaking changes may be needed, but let's go out of our way to avoid it if we can.
Vasyl Boroviak
@koresar
Apr 24 2015 01:44
open/closed principle usually ends up with bloated (big) codebase. :) But who counts?
Eric Elliott
@ericelliott
Apr 24 2015 01:44
Yeah, but there are lots of production apps with tens of millions of users depending on stampit. I don't want to break them if we don't have to. =)
We want to make a migration path to the future.
examples of apps using stampit: Adobe Creative Cloud, The Wall Street Journal World Stream, ESPN, BBC, ABC, dozens of large brand promotional sites...
and those are just the ones I know about...
I'm not saying "don't fix bugs." Just "Be careful with breaking changes, 'cause a lot of apps use it."
Vasyl Boroviak
@koresar
Apr 24 2015 01:51
I don't know what to say. :)
Eric Elliott
@ericelliott
Apr 24 2015 01:52
I trust you. Come up with good names for two new methods and let .state() do the old thing. We'll add a deprecation warning in the docs (not the code).
old stuff will probably still work (unless they depend on a bug)
Vasyl Boroviak
@koresar
Apr 24 2015 01:53
then state() should be shallow copying values. Is that what you think is the best?
Eric Elliott
@ericelliott
Apr 24 2015 01:54
yeah.
that's what .state() does in 1.x, right?
Vasyl Boroviak
@koresar
Apr 24 2015 01:55
yes
We'd need to add an alias method(s) - refs().
And one more method for deep cloning - props() ?
Eric Elliott
@ericelliott
Apr 24 2015 01:57
Yes.
Vasyl Boroviak
@koresar
Apr 24 2015 01:59
Ok. What about stamp immutability and compatibility?
Eric Elliott
@ericelliott
Apr 24 2015 01:59
ericelliott/stampit#56
Vasyl Boroviak
@koresar
Apr 24 2015 02:00
+1
Eric Elliott
@ericelliott
Apr 24 2015 02:00
I'm in favor of breaking compatibility to achieve stamp immutability. I feel that's an important change in the API. What do you think?
Vasyl Boroviak
@koresar
Apr 24 2015 02:00
I agree on that.
Eric Elliott
@ericelliott
Apr 24 2015 02:02
Awesome. =)
Thank you for all your great contributions to the project. I really appreciate your efforts, and I know the rest of the community does, too.
Vasyl Boroviak
@koresar
Apr 24 2015 02:03
Thank you for the feedback and the conversation. :) cheers
Vasyl Boroviak
@koresar
Apr 24 2015 02:10
@ericelliott should I add fourth parameter to the stampit() method for the new deep cloned props?
ericelliott/stampit#57
Carl Olsen
@unstoppablecarl
Apr 24 2015 13:02
I really like the .refs() and .props() plan. I have thought of a corner case though.
what if you want to set something by ref more than one level deep? like I want to do {connections: { db: theDbConnection } } ? I think this is kind of an anti-pattern but I am currious what the options to accomplish it would be.
In the case of property name collision between .refs() and .props() I assume last in wins?
Eric Elliott
@ericelliott
Apr 24 2015 19:16
  1. Use refs, and connections will be a ref. 2. last in should always win.
Carl Olsen
@unstoppablecarl
Apr 24 2015 19:17
What if connections.db is an instance property structure