Skip to content
This repository has been archived by the owner on Aug 28, 2022. It is now read-only.

Do not give refs to stateless function components #57

Merged
merged 1 commit into from
Sep 8, 2017

Conversation

doomsower
Copy link
Contributor

Some of my components are stateless functions, they accept style prop and pass it down to children. I want to style them with glamorous. However, I get annoying yellowbox warnings like this

Warning: Stateless function components cannot be given refs. Attempts to access this ref will fail.

Check the render method of `Icon`.
    in IconBase (created by Icon)
    in Icon (at PhotoPickerItem.js:51)

where IconBase is stateless function and Icon is glamorous(IconBase, { displayName: 'Icon' })(...). If I replace IconBase with class, those warning no longer appear.

So I added this simple condition. Glamorous component factory now checks if wrapped component is stateless function, and if so, it will not give it ref

I tries this with react-native 0.45.1, I did a quick search and I think this exactly warning message was introduced recently, so in older versions of react-native it might not be displayed or have different message.

@codecov-io
Copy link

codecov-io commented Jun 21, 2017

Codecov Report

Merging #57 into master will increase coverage by 0.05%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #57      +/-   ##
==========================================
+ Coverage   92.02%   92.08%   +0.05%     
==========================================
  Files          10       10              
  Lines         138      139       +1     
  Branches       37       38       +1     
==========================================
+ Hits          127      128       +1     
  Misses          7        7              
  Partials        4        4
Impacted Files Coverage Δ
src/create-glamorous.js 93.75% <100%> (+0.13%) ⬆️

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 3a3770e...bdf2f9e. Read the comment docs.

@hssrrw
Copy link

hssrrw commented Sep 8, 2017

This is why I have to replace my functional components with classes. I wonder what takes it so long to merge the PR.

@kwelch
Copy link
Contributor

kwelch commented Sep 8, 2017

Not sure, one thing that may help would be a test to show how it breaks and they this resolves that. Additionally, I would rebase while adding that test.

@atticoos
Copy link
Contributor

atticoos commented Sep 8, 2017

Apologies for the holdup @hssrrw, this does appear to solve the problem, but would love to see some unit tests in place. I think we should be able to merge this up and follow up with some tests verifying this change.

@atticoos atticoos merged commit 1c74dbc into robinpowered:master Sep 8, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants