statelyai / xstate

Actor-based state management & orchestration for complex app logic.
https://stately.ai/docs
MIT License
27.18k stars 1.26k forks source link

Rollup won't bundle xstate properly for browser #706

Closed eugenezastrogin closed 5 years ago

eugenezastrogin commented 5 years ago

Description Cannot use XState with rollup. I tried just about everything I could think of, simplest "hello world" browser build does not work.

Expected Result

Actual Result Uncaught ReferenceError: process is not defined

Reproduction my main.ts typescript file:

import { Machine, interpret } from 'xstate';

const toggleMachine = Machine({
  id: 'toggle',
  initial: 'inactive',
  states: {
    inactive: { on: { TOGGLE: 'active' } },
    active: { on: { TOGGLE: 'inactive' } }
  }
});

rollup configs I tried:

import resolve from 'rollup-plugin-node-resolve'; import typescript from 'rollup-plugin-typescript';

export default [{ input: './src/assets/ts/main.ts', output: { file: './build/main.js', format: 'iife', }, plugins: [ typescript(), resolve(), ] }];

This fails in runtime with Uncaught ReferenceError: process is not defined this is what leaks into bundle code:

var IS_PRODUCTION = process.env.NODE_ENV === 'production'; Setting this var in bundle manually to true seems to fix the issue though.

All browser module outputs, iife, umd and esm bundles fail with the same error.

Additional context Used versions:

"rollup": "^1.23.1",
"rollup-plugin-node-resolve": "^5.2.0",
"rollup-plugin-typescript": "^1.0.1",
"typescript": "^3.6.0",
 "xstate": "^4.6.7"
Andarist commented 5 years ago

This is a normal situation - if you consume npm libraries with rollup you are expected to use rollup-plugin-replace in your rollup config, so you can opt into development or production build. Like this:

import resolve from 'rollup-plugin-node-resolve';
import typescript from 'rollup-plugin-typescript';
import replace from 'rollup-plugin-replace';

export default [{
    input: './src/assets/ts/main.ts',
    output: {
        file: './build/main.js',
        format: 'iife',
    },
    plugins: [
        typescript(),
        resolve(),
        replace({
            'process.env.NODE_ENV': JSON.stringify('production')
        })
    ]
}];

The "issue" is not limited to XState library - you would experience this when trying to consume multitude of other frontend libraries (e.g. react, redux).

eugenezastrogin commented 5 years ago

I see... 🤔 Thank you! Might be worth adding this info to the installation section, I'll try to get around to it. I spent quite a few hours reading rollup and XState docs, thinking I missed something...

Andarist commented 5 years ago

This IMHO should be requested to the Rollup team - as it's something that affects usage of their tool (even though core Rollup doesn't care about this stuff and it's handled through plugins). That way people could find that information there and the info would be helpful there for wider audience than XState users

mreinstein commented 5 years ago

if you consume npm libraries with rollup you are expected to use rollup-plugin-replace in your rollup config, so you can opt into development or production build

Putting nodejs-specific global variables in code that could be destined for browsers or a backend, and then expecting build tools to just deal with this brokenness is pure insanity.

davidkpiano commented 5 years ago

@mreinstein Would love a PR with a proposed solution 🙏

Andarist commented 5 years ago

Putting nodejs-specific global variables in code that could be destined for browsers or a backend, and then expecting build tools to just deal with this brokenness is pure insanity.

And using npm (node.js package manager) to install dependencies destined for browsers is pure madness as well.

Sorry for the irony, but really to change this you would have to change the whole community's standard for dev/prod differential building and provide an alternative way of doing this that would be implemented by at least most of the current popular tooling without sacrificing both DX and final bundle sizes.

Using a bundler as is already step aside from the old ways of shipping an application and I fail to see how this particular (process.env.NODE_ENV) thing differs from all the other stuff that bundlers are doing. Once you start bundling you already need to configure bunch of stuff to handle everything properly for you. There are even "browser" redirects in package.json which makes bundlers to chose browser-friendly files over node.js-specific ones, like here, because "main" of the package.json (the default) is destined for node.js (or isomorphic stuff, if for a particular package the environment doesn't matter) as the whoel thing (npm + package.json were created for node). Adding to that - when using Rollup and its rollup-plugin-node-resolve you need to opt-in into resolving from "browser" as well. It's their philosophy (which I respect), but it's different from webpack & parcel which both do that automatically for you without extra configuration.

To sum it up - npm package is not usable as is by a browser anyway. We have browser-ready files for you if that's your cup of tea:

mreinstein commented 5 years ago

And using npm (node.js package manager) to install dependencies destined for browsers is pure madness as well.

If one doesn't bake in node.js specific globals into their codebase, that would remove the mandate that said code be built through node/npm. :)

Sorry for the irony, but really to change this you would have to change the whole community's standard for dev/prod differential building

That's not an example of irony. :) Emphasis belongs on "the whole community". We're basically talking about the webpack community.

To sum it up - npm package is not usable as is by a browser anyway.

The high quality ones are. :)

to be clear I'm not singling out xstate as being the only group/package doing this. It's just a really lamentable trend in javascript packages.

Andarist commented 5 years ago

That's not an example of irony. :)

That was referring to the previous statement about "pure madness" 😉

Emphasis belongs on "the whole community". We're basically talking about the webpack community.

Which is a majority from what I can tell and also different bundlers have adopted this strategy as well, so at this point in time this is not webpack-specific. I'm also not sure if this idea has originated in webpack or not.

The high quality ones are. :)

I wouldn't call e.g. React, Redux, Vue and others low-quality.

to be clear I'm not singling out xstate as being the only group/package doing this. It's just a really lamentable trend in javascript packages.

I admit the situation is not ideal, but this is mainly because there is no real standard, no committee etc (which is both good and bad, different advantages/disadvantages for both sides of that spectrum) and such half-baked "standards" have just emerged. Better solutions can be figured out - but there is no champion for it right now, so we are stuck in this for a better or worse until one comes, figures this out and push the whole community to better ways.

Andarist commented 5 years ago

I believe this has been answered and there is no further discussion taking place here, so I'm going to close this issue.