Doesn’t seem to receive new renderer when replaced

[alanhogan]

Version: 6.0.0 (5.2.0 also)
Environment: DOM
Type: Bug

---

Description

It doesn't seem like the Fela Provider component updates its renderer when the renderer prop is updated.

Steps to reproduce

import React from 'react';

// Fela
import { Provider as FelaProvider } from 'react-fela';
import { createRenderer as createFelaRenderer } from 'fela';
import dynamicPrefixer from 'fela-plugin-dynamic-prefixer';
import fallbackValue from 'fela-plugin-fallback-value';
import rtl from 'fela-plugin-rtl';

export default class Providers extends React.Component {
  // Commented out to be sure this is not the source of the bug
  // // Need to generate a new Fela renderer for RTL vs LTR.
  // shouldComponentUpdate(nextProps, nextState) {
  //   return nextProps.rtl !== this.props.rtl;
  // }

  render() {
    const felaPlugins = [dynamicPrefixer({
        userAgent: navigator.userAgent,
        keepUnprefixed: true
      }),
      fallbackValue()
    ];

    if (this.props.rtl) felaPlugins.push(rtl());

    return (
      <FelaProvider renderer={createFelaRenderer({plugins: felaPlugins})}>
        {this.props.children}
      </FelaProvider>
    );
  }
}

// Then render the Providers component with dynamic rtl property
<Providers rtl={dynamic}>

Expected Behavior

RTL should be enabled or disabled as the appropriate property is toggled.

Actual Behavior

it does not. This is despite felaPlugins definitely gaining an additional item when rtl is true. And the rtl() plugin works if the initial state of the RTL variable is true (and then it keeps working when the RTL variable is false) or if it's hard-coded in the plugin array.

[robinweser]

Hmm, can you give me some environment to test that? Apart from that, can you try to replace only the plugins instead of the whole renderer (you will benefit from cached rules) and use this.forceUpdate()?

[alanhogan]

Hmm, I didn’t know I could dynamically change the plugins used by the renderer. I will have to look into that.

As far as an place to test this: I will probably add a “toy” demo project illustrating this in a folder of my web-experiments repo, unless you know a good way to add modules and everything to JSFiddle or a similar site.

[TxHawks]

You can try https://www.webpackbin.com

[alanhogan]

OK, fully 'working' demo: https://www.webpackbin.com/bins/-Kul3pKMx3TnV6fMBCfg

As described, toggling RTL re-renders with an additional plugin, but no difference is observed. I think the bin also includes enough to demonstrate that there 'should' be a difference (or, at least, an error message from Fela telling me that swapping in a renderer for a new one is a stupid idea and I should do something else instead)

[alanhogan]

By maintaining a the same renderer but setting the plugins property, I was able to do what I was trying to do originally: https://www.webpackbin.com/bins/-KumIr9tpOdzqccC-rMs
And of course, slightly less stupid is directly pushing/poping the plugins array on the renderer: https://www.webpackbin.com/bins/-KumL3WRNGQGKkw3L8iu

[alanhogan]

I didn't see any mention of updating a renderer's plugins in the docs, FYI.

[robinweser]

I will probably add some information to the docs, but after all the renderer is just an object with properties. You can switch everything while the cache can be reused most likely.

[alanhogan]

What of the original bug though? Should the FelaProvider component detect when someone tries to replace the entire renderer?

[robinweser]

Not sure if one should be able to replace the renderer. Feels kinda wrong to do that^^

[alanhogan]

Totally get that. It would be nice for there to be an error if some dummy like me tries to do it.

[robinweser]

(y) Will add that