store icon indicating copy to clipboard operation
store copied to clipboard

Dot separated selectors

Open jaripekkala opened this issue 7 years ago • 11 comments

This is a...

  • [x] feature request
  • [ ] bug report
  • [ ] usage question

Expected Behaviour:

@select('db.customers') customers: Observable<Customer[]>;

Actual Behaviour:

@select(['db', 'customers']) customers: Observable<Customer[]>;

Additional Notes:

interface AppState {
  // ...
  db: {
    // ...
    customers: Customer[];
  };
}

jaripekkala avatar May 03 '17 12:05 jaripekkala

FYI - i've seen project library's like selectn to support such syntax.

murraybauer avatar May 04 '17 00:05 murraybauer

My concern with this feature is that 'db.customers' is a perfectly legal property name. What happens if my store looks like this:

{
   users: {
     'seth.davenport': {
       age: 35,
       'height.cm': 170
     }
}

We would now have introduced an ambiguity in the path selector that wasn't there before.

Also, because I can't guarantee that none of our users are doing this, adding this feature would be a breaking change and would require a major version change.

SethDavenport avatar May 04 '17 03:05 SethDavenport

Thinking how common dots in the property names are I think they should be the ones making ugly selectors.

@select(['users', 'seth.davenport']) user: Observable<User[]>;

No need to hurry the next major release with this.

jaripekkala avatar May 04 '17 07:05 jaripekkala

Hmm ... @e-schultz thoughts? Is this something to consider for 7.0.0?

SethDavenport avatar May 06 '17 23:05 SethDavenport

Been thinking about this, and agree with concerns of potentially being a breaking change for some users. When going with the array style vs the . style paths - I was considering that { 'x.y' : 'thing' } is valid, and also Immutable and Ramda both use the array-style for their getIn/path.

I'll give this a bit more thought

e-schultz avatar May 08 '17 15:05 e-schultz

Maybe a injectable service that allows enabling/disabling this syntax? Or specify a middleware for the selector before passing it to the getIn?

If you decide not to support this maybe exporting a helper function from the same package to import it easy.

https://github.com/facebook/immutable-js/issues/757#issuecomment-210705453

Edit: There was a something about performance https://jsperf.com/getin-split-array/1

jaripekkala avatar May 08 '17 16:05 jaripekkala

@salomoni I like the suggestion. Although, have you considering just having a utility method that simply splits on periods? I would prefer the syntax you had suggested but I can understand why it might not be done.

jspengeman avatar May 10 '17 23:05 jspengeman

@jspengeman Really doesn't make much a difference between the current array style. The goal here is just to make the selector cleaner and easier on eyes.

@select('db.customers'.split('.')) customers: Observable<Customer[]>;

@select(split('db.customers')) customers: Observable<Customer[]>;

@select(['db', 'customers']) customers: Observable<Customer[]>;

@select('db.customers') customers: Observable<Customer[]>;

Adding on the performance: https://jsperf.com/getinsplit-check/1 Makes almost no difference at all to check if you decide to use the current array style.

jaripekkala avatar May 11 '17 08:05 jaripekkala

maybe try

@select('user.username'.split('.'))
username: Observable<string>;

naivefun avatar Jul 26 '17 04:07 naivefun

If users really have a syntax like { 'seth.davenport': {} } as mentioned above the user have to access to the property like that: users['seth.davenport']. So the user might think he will get the property like following:

@select(`users['seth.davenport']`) user: Observable<User[]>;

I don't think it would be a breaking change, because it is just a new feature to the select method, and will not break any current behavior, or am I wrong?

JPeer264 avatar Aug 08 '17 06:08 JPeer264

Even though there is no guarantee that any chars would be a good candidate (apart from weird ones such as ~) what about a less sensitive char /?

@select('db/customers')
customers$: Observable<Customer[]>;

aminpaks avatar Oct 14 '17 20:10 aminpaks