Reset roolEl when calling withComponent

[tikotzky]

What:
A fix and failing test for #376

Why:
Added as per #376 (comment)

How:
When calling withComponent the rootEl is now set to the component being passed in.
You can still manually set the rootEl by passing in options in the second param.

Checklist:

• Documentation N/A
• Tests
• Ready to be merged
• Added myself to contributors table

[codecov-io]

Codecov Report

Merging #377 into master will not change coverage.
The diff coverage is n/a.

@@          Coverage Diff          @@
##           master   #377   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files          10     10           
  Lines         182    182           
  Branches       52     52           
=====================================
  Hits          182    182

Impacted Files	Coverage Δ
src/create-glamorous.js	100% <ø> (ø)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b564686...6f840a4. Read the comment docs.

[kentcdodds]

Looks good! Just one more case to satisfy :) Thanks so much!

[kentcdodds]

This should probably be rootEl: getRootEl(newComp) just in case someone passes withComponent(SomeGlamorousComponent) 👍

[tikotzky]

done

[marzelin]

hmm... getRootEl might do more harm than help there. Could you try if this example works for your code? (bigger font size should be applied) https://codesandbox.io/s/pkr1rn24vx

[kentcdodds]

Hmm... I'm not sure I understand. I think your codesandbox simply reveals the bug that would be fixed by these changes.

[marzelin]

@kentcdodds I didn't have time to explain this properly yesterday, so here it goes:

Normally, when you pass a glamorous component to a glamourous(), it 'merges' all the glamorous props so that there is only one instance of a glamorous component. withComponent works differently - when you pass a glamorous component to it there will be two instances - one glamorous instance rendering the other.

rootEl purpose is to decide what props should be filtered out (props that are invalid for a given rootEl won't be passed to the rendered component). When you assign rootEl to a wrapped non-glamorous component (mosty html element) all custom props will be filtered out.

This causes problems when this inner glamorous component (the one given to withComponent) relies on custom props for dynamic styles (it won't be able to access the custom props).

In summary: when passing glamorous component to withComponent, the component that will be directly rendered is the glamorous component itself so it should be assigned to rootEl.

tldr;
this works in 4.11.2 https://codesandbox.io/s/pkr1rn24vx
but doesn't in the latest (4.11.4) https://codesandbox.io/s/n0yrxyrnl

[kentcdodds]

I think I understand. Could you perhaps open a pull request which adds a test for your scenario and see if we can satisfy both cases? If not then we can have a discussion about which to favor...

[tikotzky]

@kentcdodds one other thing...
this test currently is not testing anything since .find('div') is not finding the div.
This means that this test will always pass even if the props are getting set.

I can fix it by removing .find('div'), but the better fix would be to switch to a snapshot test as well, this way you would not have to worry about it wrongly being undefined in the future.
Should I switch my test and the above test to use snapshots?

[kentcdodds]

Thanks for catching that. Let's go ahead and merge this and you can fix that test in another PR if you like. I like to try to avoid snapshots if I can, preferring a more explicit assertion. So if you're able to come up with something more explicit it would be appreciated. Thanks!

[kentcdodds]

Wonderful! Thank you for fixing this!

[kentcdodds]

Thanks so much for your help @tikotzky! I've added you as a collaborator on the project. Please make sure that you review the other/MAINTAINING.md and CONTRIBUTING.md files (specifically the bit about the commit messages and the git hooks) and familiarize yourself with the code of conduct (we're using the contributor covenant). You might also want to watch the repo to be notified when someone files an issue/PR. Please continue to make PRs as you feel the need (you can make your branches directly on the repo rather than your fork if you want). Thanks! And welcome to the team :)