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

Rollup won't bundle xstate properly for browser #706

Closed
eugenezastrogin opened this issue Oct 8, 2019 · 9 comments
Closed

Rollup won't bundle xstate properly for browser #706

eugenezastrogin opened this issue Oct 8, 2019 · 9 comments

Comments

@eugenezastrogin
Copy link

eugenezastrogin commented Oct 8, 2019

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"
@eugenezastrogin eugenezastrogin changed the title Won't import correct bundle with rollup Rollup won't bundle xstate properly for browser Oct 8, 2019
@Andarist
Copy link
Member

Andarist commented Oct 8, 2019

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
Copy link
Author

eugenezastrogin commented Oct 8, 2019

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
Copy link
Member

Andarist commented Oct 8, 2019

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

@Andarist Andarist removed the bug label Oct 8, 2019
@mreinstein
Copy link
Contributor

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
Copy link
Member

@mreinstein Would love a PR with a proposed solution 🙏

@Andarist
Copy link
Member

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
Copy link
Contributor

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
Copy link
Member

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
Copy link
Member

Andarist commented Nov 7, 2019

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

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

4 participants