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

How to work around React errors (calling direct / unique key) #48

Open
olivergeorge opened this issue Jan 22, 2015 · 3 comments
Open

How to work around React errors (calling direct / unique key) #48

olivergeorge opened this issue Jan 22, 2015 · 3 comments

Comments

@olivergeorge
Copy link

I'm building a select field, using Sablono I get React errors. I'm struggling to see how to avoid them short of completely switching to using pure om controls.

This is my problem code:

(defn Select [props owner]
  (om/component
    (let [{:keys [label value on-change help placeholder disabled
                  options default-option default-value]} props
          default-value (or default-value "")
          default-option (or default-option "Please select")]
      (html [:div.form-group
             (if label [:label label])
             [:select.form-control {:value       (or value default-value)
                                    :placeholder placeholder
                                    :disabled    disabled
                                    :on-change   on-change}
              (if (and options)
                [:option {:key default-value :value default-value :disabled true} default-option])
              (om/build-all Option options)]
             (if help [:p.help-block help])]))))

I get three react warnings:

  1. Warning: Something is calling a React component directly. Use a factory or JSX instead. See: http://fb.me/react-legacyfactory
  2. Each child in an array should have a unique "key" prop. Check the renderComponent call using . See http://fb.me/react-warning-keys for more information.
  3. Warning: transferPropsTo is deprecated. See http://fb.me/react-transferpropsto for more information.

And this is what I ended up with to avoid the problem, note use of (apply dom/select...).

(defn Select [props owner]
  (om/component
    (let [{:keys [label value on-change help placeholder disabled
                  options default-option default-value]} props
          default-value (or default-value "")
          default-option (or default-option "Please select")]
      (html [:div.form-group
             (if label [:label label])
             (apply dom/select #js {:className   "form-control"
                                    :value       (or value default-value)
                                    :placeholder placeholder
                                    :disabled    disabled
                                    :onChange   on-change}
                    (if options
                      (dom/option #js {:value default-value :disabled true} default-option))
                    (om/build-all Option options))
             (if help [:p.help-block help])]))))

My feeling is that I'm missing some standard sablono tricks for handling the case of many children in a vector.

@olivergeorge
Copy link
Author

Quick follow up, this approach got me a bit closer. I use (vec (concat ...)) to package up the options.

(defn Option [props owner]
  (om/component
    (let [[value display] props]
      (dom/option #js {:value value} display))))

(defn Select [props owner]
  (om/component
    (let [{:keys [label value on-change help placeholder disabled
                  options default-option default-value]} props
          default-value (or default-value "")
          default-option (or default-option "Please select")]
      (html [:div.form-group
             (if label [:label label])
             (vec (concat
                     [:select.form-control {:value       (or value default-value)
                                            :placeholder placeholder
                                            :disabled    disabled
                                            :on-change   on-change}
                     (if options
                        (dom/option #js {:value default-value :disabled true} default-option))]
                     (om/build-all Option options)))
             (if help [:p.help-block help])]))))

Note: I still have to use (dom/option...) to avoid two of the errors:

@the-kenny
Copy link
Contributor

Oliver George [email protected] writes:

Quick follow up, this approach got me a bit closer. I use (vec (concat ...)) to package up the options.

(defn Option [props owner]
  (om/component
    (let [[value display] props]
      (dom/option #js {:value value} display))))

(defn Select [props owner]
  (om/component
    (let [{:keys [label value on-change help placeholder disabled
                  options default-option default-value]} props
          default-value (or default-value "")
          default-option (or default-option "Please select")]
      (html [:div.form-group
             (if label [:label label])
             (vec (concat
                     [:select.form-control {:value       (or value default-value)
                                            :placeholder placeholder
                                            :disabled    disabled
                                            :on-change   on-change}
                     (if options
                        (dom/option #js {:value default-value :disabled true} default-option))]
                     (om/build-all Option options)))
             (if help [:p.help-block help])]))))

Note: I still have to use (dom/option...) to avoid two of the errors:


Reply to this email directly or view it on GitHub:
#48 (comment)

Those are caused by implementation details of Sablono/Om. They will
likely disappear in future releases of those.

Your 'fixed' error messages is caused by the 'list' of members in the
div.form-group: React needs key-attributes so it doesn't have to redraw
the whole list in case the order changes. With `om/build', this is done
with :key or :react-key. Inside components, Om seems to support (or
leak?) :key with a value.

While this is a bit unintuitive given the naming of options to
`om/build', it can be used to work around your warning: Just give every
child-node of div.form-group an unique key like:

[:label {:key "label"}]
[:select.form-control {:key "my-select"}]

etc.

I'm not sure if this is intended in Om: I suspect it would be
better to factor out those elements as their own components and call
`om/build' on them.

@olivergeorge
Copy link
Author

Thanks, I'll give that a go.

Might be worth having a wiki page noting common causes/resolutions
associated with react warnings.

On 22 January 2015 at 20:19, Moritz Ulrich [email protected] wrote:

Oliver George [email protected] writes:

Quick follow up, this approach got me a bit closer. I use (vec (concat
...)) to package up the options.

(defn Option [props owner]
(om/component
(let [[value display] props]
(dom/option #js {:value value} display))))

(defn Select [props owner]
(om/component
(let [{:keys [label value on-change help placeholder disabled
options default-option default-value]} props
default-value (or default-value "")
default-option (or default-option "Please select")]
(html [:div.form-group
(if label [:label label])
(vec (concat
[:select.form-control {:value (or value default-value)
:placeholder placeholder
:disabled disabled
:on-change on-change}
(if options
(dom/option #js {:value default-value :disabled true} default-option))]
(om/build-all Option options)))
(if help [:p.help-block help])]))))

Note: I still have to use (dom/option...) to avoid two of the errors:


Reply to this email directly or view it on GitHub:
#48 (comment)

Those are caused by implementation details of Sablono/Om. They will
likely disappear in future releases of those.

Your 'fixed' error messages is caused by the 'list' of members in the
div.form-group: React needs key-attributes so it doesn't have to redraw
the whole list in case the order changes. With `om/build', this is done
with :key or :react-key. Inside components, Om seems to support (or
leak?) :key with a value.

While this is a bit unintuitive given the naming of options to
`om/build', it can be used to work around your warning: Just give every
child-node of div.form-group an unique key like:

[:label {:key "label"}]
[:select.form-control {:key "my-select"}]

etc.

I'm not sure if this is intended in Om: I suspect it would be
better to factor out those elements as their own components and call
`om/build' on them.


Reply to this email directly or view it on GitHub
#48 (comment).

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

No branches or pull requests

2 participants