RickWong/react-transmit

Initial Variables required unnecesarily

Open

#57 opened on Feb 12, 2016

View on GitHub
 (3 comments) (0 reactions) (0 assignees)JavaScript (1,320 stars) (71 forks)batch import
help wanted

Description

First of all, nice project! I want to point out an issue I found recently. I tried implementing this in one of my projects but the Container wouldn't render so I ended up tracing the error to the componentWillMount lifecycle method of the Container, specifically to this line: https://github.com/RickWong/react-transmit/blob/master/src/lib/createContainer.js#L239. Going down that function call I noticed that this function returns false if the isRootContainer property of the Container is false, which in my case was because I hadn't declared initialVariables (https://github.com/RickWong/react-transmit/blob/master/src/lib/createContainer.js#L30). Once I set initialVariables to {} it worked as expected, but in my humble opinion that should have been uneccesary as I don't use any variables in my fragment functions. Or is this behaviour intended?

EDIT: Even if this behaviour is intended I don't think it should fail silently.

Contributor guide

Tech stack
javascriptreact
Domain
frontend
Issue type
bug
DifficultyEstimated implementation difficulty for a new contributor, from 1 for very small changes to 5 for expert-level work.
2
Estimated timeA rough time range for an experienced contributor to investigate, implement, test, and prepare a pull request.
under 1 hour
Activity statusHow available the issue appears right now: fresh, active, stale, blocked, or waiting on maintainer input.
stale
ClarityHow clearly the issue explains the expected change, acceptance criteria, and next step.
clear
Prerequisites
basic React knowledge
Newbie friendlinessA 1-100 score estimating how approachable this issue is for first-time contributors.
70
Research direction
Investigate the `componentWillMount` and the function around line 239 in `createContainer.js`. The issue is that `initialVariables` is required even when not needed. A fix would involve checking if `initialVariables` is undefined and treating it as an empty object, or adding a default. Also consider the related check on line 30 that determines `isRootContainer`. The current behavior silently fails; a warning or error could be added.