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

useResponsiveComponent prop values not applied during SSR #272

Closed
kettanaito opened this issue Dec 28, 2019 · 5 comments · Fixed by #281
Closed

useResponsiveComponent prop values not applied during SSR #272

kettanaito opened this issue Dec 28, 2019 · 5 comments · Fixed by #281
Assignees
Labels
bug good first issue Good place to start for newcomers help wanted Extra attention is needed in progress needs:tests This issue needs tests. Desperately. priority:high This issue violates the specification scope:calculation This issue relates to the calculation logic

Comments

@kettanaito
Copy link
Owner

kettanaito commented Dec 28, 2019

What:

import styled from 'styled-components'
import { useResponsiveComponent } from 'atomic-layout'

const StyledImage = styled.img`
  border-radius: 5px;
`
const ElementImage = useResponsiveComponent(StyledImage)

const Example = () => (
  <ElementImage src="foo.png" />
)

The ElementImage component will render an image without the src attribute during SSR.

Why:

My initial guess is that useResponsiveComponent, has its values throttled in a way which results the initial value not being set at all.

Environment:

  • node vesrion: 12.x
  • npm version: 6.x
  • atomic-layout version: 0.12.x
@kettanaito kettanaito added bug help wanted Extra attention is needed good first issue Good place to start for newcomers needs:tests This issue needs tests. Desperately. scope:calculation This issue relates to the calculation logic priority:high This issue violates the specification labels Dec 28, 2019
@kettanaito
Copy link
Owner Author

Would you have time to look into this issue please, @ruhaise? I'd gladly provide with assistance along the way.

@ruhaise ruhaise self-assigned this Dec 29, 2019
@ruhaise
Copy link
Collaborator

ruhaise commented Dec 29, 2019

@kettanaito im on it :)

@kettanaito
Copy link
Owner Author

@ruhaise Thanks! I can recommend to start from writing a unit test for SSR of useResponsiveComponent use case described above. Take a look at the SSR unit test for inspiration. Perhaps we would have to extend that test suite to render responsive component for verification of this issue.

Seeing a relevant test failing is a great first step to start fixing anything.

@ruhaise
Copy link
Collaborator

ruhaise commented Dec 29, 2019

@kettanaito thanks for the advice, sounds promising

@kettanaito
Copy link
Owner Author

Initial fix published in [email protected]. Please update for it to take effect.

There is a new behavior-related issue found and will be addressed under #284.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug good first issue Good place to start for newcomers help wanted Extra attention is needed in progress needs:tests This issue needs tests. Desperately. priority:high This issue violates the specification scope:calculation This issue relates to the calculation logic
Projects
None yet
2 participants