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

More granular query methods with predicate function and/or props #9

Open
jrunning opened this issue Sep 18, 2015 · 13 comments
Open

More granular query methods with predicate function and/or props #9

jrunning opened this issue Sep 18, 2015 · 13 comments

Comments

@jrunning
Copy link

It would be great to be able to do more granular searches by passing a props or function argument to query methods):

// Given the following tree:
<div>
  <Widget title="First" count="10" active />
  <Widget title="Second" count="20"/>
  <Widget title="Third" count="30"/>
</div>

tree.findComponent('Widget', { title: 'First' });
// => <Widget title="First" .../>

tree.findComponent('Widget', { active: false });
// => <Widget title="Second" .../>

tree.findComponent('Widget', (props) => props.count > 25);
// => <Widget title="Third" .../>

On a related note, it would be nice if there was an everySubTreeLike method analogous to findComponentLike and more "plural" methods to return multiple nodes/components e.g. findNodes/findComponents(Like)/whathaveyou.

@glenjamin
Copy link
Owner

Hi!

Thanks for the feedback.

The current inconsistent APIs are mostly a result of evolution from the original API and various holes we found via internal use. Every time I added a feature I kept backwards-compatibility with the previous stuff.

I'm not entirely sure that all the existing APIs make sense.

I like the idea of being able to pass a filter function as well.

There's also a semi-hidden chai plugin you can require as skin-deep/chai, which can lead to some nicer assertions.

At the moment there's 3 different families:

  • subTree - get another SkinDeep object by CSS selector
  • findNode - get a node by CSS selector
  • findComponent - get a node by component/props

And currently subTree has the every variant, and component has the like variant.

This seems a bit muddled - internally the implementation is all just a single tree-walk function which takes a predicate - so any combination of the above is pretty doable.

We could just expand all 3 flavours so they support all the variants, but ideally I'd like to trim down a bit.

@glenjamin
Copy link
Owner

Here's some stats from our 3 internal testsuites - I'll ask around the office as well and see what people think.

grep -hriEo 'findComponent(like)?|(every)?subTree|findNode' -- test | sort | uniq -c | sort -n
   4 findComponent
  10 everySubTree
  36 findComponentLike
  45 subTree
  86 findNode
   2 subTree
   5 findComponentLike
 101 findNode
   2 findComponentLike
   2 subTree
  63 findNode

The skew towards findNode is likely because that was the original API, rather than it being any better.

@jrunning
Copy link
Author

Thanks, that's good info. I agree that the API could use some refinement.

One thing about my use case is that my app has very few ids or classNames (we use react-css-modules, so we have mostly styleNames), so I found findNode to be of limited utility.

@glenjamin
Copy link
Owner

We've actually been putting in specific classes for our end-to-end selenium tests, and then hooking some unit tests of those, eg .qa-login-button. We're starting to move away from this for unit tests towards more subTree and findComponent based approaches.

@glenjamin
Copy link
Owner

One thing I've noticed we do a lot is findNode('ComponentName') and then assert on props - this doesn't help if you have many components, but does give better errors if there's one.

@glenjamin
Copy link
Owner

We had a chat internally, current thinking is to trim down to

  • everySubTree
  • everySubTreeLike
  • subTree
  • subTreeLike

And expand these so they all take a selector as the first argument, and a props object / predicate as the second argument.

@glenjamin
Copy link
Owner

glenjamin commented Oct 6, 2015

Summary of current plan

  • Expand subTree functions to accept (selector, props).
  • Expand subTree functions to accept (selector, predicate).
  • Add (every)subTreeLike functions
  • Allow passing an array as a selector
  • Expose props on the SkinDeep object
  • Add deprecation warnings to every non subTree function

@jrunning
Copy link
Author

jrunning commented Oct 6, 2015

That sounds great!

@glenjamin
Copy link
Owner

I'm working on writing up notes for a v1.0 API - what do people think about changing subTree to do what subTreeLike currently does, and adding a subTreeExact function?

The idea is to change the API to push users towards loose selectors over strict ones.

/cc @jrunning @hoegrammer @foiseworth @donabrams @hartzis

@glenjamin
Copy link
Owner

A possible approach would be to use the fact that the new API will accept a function as the second argument for the overloading:

// match subset of props
tree.subTree('Component', { some: 'prop' });

// match exact props
tree.subTree('Component', sd.exact({exact: 'props'}));

This would allow trimming down the top-level selection API to only subTree and everySubTree

@glenjamin
Copy link
Owner

I've posted a draft of introduction and overview docs for what would become the 1.0 API: https://github.com/glenjamin/skin-deep/tree/one-point-oh

@hartzis
Copy link
Contributor

hartzis commented Nov 16, 2015

Definitely agree with the change to subTree to match functionality of subTreeLike, and everything else you're proposing in the docs for 1.0!

Have you thought about dropping support for react 0.13 when jumping to v1.0?

@glenjamin
Copy link
Owner

I thought about dropping 0.13, but we've not upgraded at work yet, and the compat layer is quite thin so I'll probably keep it.

Might do 0.99 and save 1.0 for when 0.13 drops

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants