sablono icon indicating copy to clipboard operation
sablono copied to clipboard

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

Open olivergeorge opened this issue 10 years ago • 3 comments

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 avatar Jan 22 '15 06:01 olivergeorge

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:

  • Warning: Something is calling a React component directly. Use a factory or JSX instead. See: http://fb.me/react-legacyfactory
  • Warning: transferPropsTo is deprecated. See http://fb.me/react-transferpropsto for more information.

olivergeorge avatar Jan 22 '15 07:01 olivergeorge

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:

  • Warning: Something is calling a React component directly. Use a factory or JSX instead. See: http://fb.me/react-legacyfactory
  • Warning: transferPropsTo is deprecated. See http://fb.me/react-transferpropsto for more information.

Reply to this email directly or view it on GitHub: https://github.com/r0man/sablono/issues/48#issuecomment-70979653

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.

the-kenny avatar Jan 22 '15 09:01 the-kenny

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:

  • Warning: Something is calling a React component directly. Use a factory or JSX instead. See: http://fb.me/react-legacyfactory
  • Warning: transferPropsTo is deprecated. See http://fb.me/react-transferpropsto for more information.

Reply to this email directly or view it on GitHub: https://github.com/r0man/sablono/issues/48#issuecomment-70979653

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 https://github.com/r0man/sablono/issues/48#issuecomment-70992047.

olivergeorge avatar Jan 22 '15 20:01 olivergeorge