developit / unistore

🌢 350b / 650b state container with component actions for Preact & React
https://npm.im/unistore
2.86k stars 139 forks source link

Preact components fail to update when loading Preact as UMD external dep and Unistore as a part of the bundle #59

Open rinatkhaziev opened 6 years ago

rinatkhaziev commented 6 years ago

First off, thanks, your stuff kicks some serious ass! πŸ‘πŸ»

Now to the issue -

I've been using Preact lately to build all sorts of widgets, they're not parts of the same app, and completely unrelated to each other, however, several widgets can be loaded on the same page. To avoid including Preact in each widget's bundle I opted to use UMD build from unpkg. The approach worked great until yesterday, when I tried to implement Unistore. I loaded react as a UMD module, and bundled Unistore with the app. Preact component fails to get updated when Unistore updates state. Here's sample code, I removed all parts of actual app to make sure it doesn't work because I've done something stupid. I couldn't reproduce on CodeSandbox, it errors out with DependencyNotFoundError - https://codesandbox.io/s/qzn378wx06

The build happens via rollup.

The issue doesn't arise if both Preact/Unistore are loaded as UMD or bundled.

Here's the screencast: http://recordit.co/OShYYc3DSm

And here's the code: main.js

let { h, render, Component } = preact;
import createStore from 'unistore'
import { Provider, connect } from 'unistore/preact'

const initialState = {
  dateFilterExpanded: false,
}

export const store = createStore(initialState)

let actions = store => ({
  toggleCb: ({dateFilterExpanded}, event) => { console.log(dateFilterExpanded, event); return { dateFilterExpanded: ! dateFilterExpanded } },
  changeCb: (state, event) => { console.log(event); store.setState({ [event.target.name]: event.target.value }) }
});

@connect(Object.keys(initialState), actions)
export class App extends Component {
  render({dateFilterExpanded, changeCb, toggleCb}, state) {
    return <a onClick={toggleCb}>I will not change <strong>{ `dateFilterExpanded: ${dateFilterExpanded}`}</strong></a>
  }
}

const initWidget = (selector = 'body', props = {}) => {
    render( <Provider store={store}><App /></Provider>, document.querySelector( selector ) )
}

initWidget('#attribution-totals-widget')

index.html

<!DOCTYPE html>
<html lang="en-US">
  <head>
    <title></title>
    <meta charset="UTF-8">
    <meta http-equiv="X-UA-Compatible" content="IE=edge">
    <meta name="viewport" content="width=device-width, initial-scale=1">

    <link rel="stylesheet" href="style.css">
  </head>
  <body>
    <div id="attribution-totals-widget" class="attribution-totals-widget"></div>
    <script src="https://unpkg.com/preact@8.2.7/dist/preact.min.js"></script>
    <script src="bundle.js"></script>
  </body>
</html>
developit commented 6 years ago

Hi there - you're getting 2 copies of Preact, which is unsupported. You'll either need to externalize Preact in your bundle, or tell Unistore to look at your bundled copy of Preact.

rinatkhaziev commented 6 years ago

Hi,

How am I getting 2 copies of Preact? I thought it gets properly externalized on bundle:

const read = {
  entry: 'src/js/main.js',
  sourceMap: true,
  external: [
    'preact',
    'preact/devtools',
  ],
  plugins: [
    resolve({ jsnext: true, main: true }),
    commonjs(),
    babel({ exclude: 'node_modules/**', runtimeHelpers: true }),
    uglify(),
    replace({ 'process.env.NODE_ENV': JSON.stringify('development') })
  ]
}
const write = {
  format: 'iife',
  sourceMap: true,
  globals: {
    preact: "preact",
  },
}

rollup
    .rollup(read)
    .then(bundle => {
      // generate the bundle
      const files = bundle.generate(write)

      // write the files to dist
      fs.writeFileSync('dist/bundle.js', files.code)
      fs.writeFileSync('dist/maps/bundle.js.map', files.map.toString())
    })

Am I missing something?

developit commented 6 years ago

Hmm - preact has a jsnext:main and module entries, perhaps those aren't getting picked up by your external mapping?

Try using an ESM import for Preact rather than the const statement, to make sure you're getting the same copy:

import { Component } from 'preact';

console.log(Component===preact.Component)
rinatkhaziev commented 6 years ago

when I do it through import, Component===preact.Component is true

However, transformation of JSX class components produces wrong results

// app.jsx
@connect( 'items', actions )
class TestComponent extends Component {
  render() {
    return <div>test</div>
  }
}

// bundle.js
var TestComponent = (_dec = connect('items', actions), _dec(_class = function (_Component) {
  _inherits(TestComponent, _Component);

  function TestComponent() {
    _classCallCheck(this, TestComponent);

    return _possibleConstructorReturn(this, (TestComponent.__proto__ || Object.getPrototypeOf(TestComponent)).apply(this, arguments));
  }

  _createClass(TestComponent, [{
    key: "render",
   // Note that instead of render() the function is named preact.render() which is invalid
    value: function preact.render() {
      return preact.h(
        "div",
        null,
        "test"
      );
    }
  }]);

I assume it's an issue with transform-react-jsx, but it works perfectly fine when I do let { h, render, Component } = preact

Here's my .babelrc


{
  "presets": [
    [
      "es2015",
      {
        "modules": false
      }
    ],
    "stage-0"
  ],
  "plugins": [
    [
      "transform-runtime",
      {
          "polyfill": false,
          "regenerator": true
      }
    ],
    "transform-decorators-legacy",
    "external-helpers",
    [
      "transform-react-jsx",
      {
        "pragma": "h"
      }
    ]
  ]
}```
developit commented 6 years ago

Strange - seems like a Rollup bug?

rinatkhaziev commented 6 years ago

Yeah, I'm not sure, probably Rollup's, but I gave up on that investigation.

I tested with a functional component, it compiles fine, however the issue is still there. I tried both let { h, render, Component } = preact

and import { h, render, Component } from 'preact'

console.log(Component===preact.Component) is true in both cases, but the issue is still there.

mindplay-dk commented 6 years ago

Is this why I can't get a basic example working on codesandbox at all?

https://codesandbox.io/s/9zk1po55ly

I've tried everything I could think of - it refuses to import the createStore function, from either unistore or unistore/full/preact.

herrKlein commented 2 years ago

solved by using commonjs({transformMixedEsModules:true}),
in rollup build script. preact was imported in our project with an import statement, and in the unistore/preact build with a require. This lead rollup probably think 2 different versions were used and bundled preact 2 times. By adding 'transformMixedEsModules:true' to the commonjs importer of rollup this problem was solved.