Check if `process` is defined

[davidkpiano]

This PR checks if process is defined before reading it. I ran into this when trying to use XState with Cloudflare Durable Objects.

Fixes #787

[changeset-bot[bot]]

⚠️ No Changeset found

Latest commit: b3428049aed78e7302397bf4d2a066abf6896c4d

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

[ghost]

CodeSee Review Map:

Review in an interactive map

View more CodeSee Maps

Legend

[Andarist]

How have you tried to consume XState in this environment? Have you used any of the templates mentioned here?

[davidkpiano]

How have you tried to consume XState in this environment? Have you used any of the templates mentioned here?

Yes, I used the TS template. I think it's a good idea to not assume that process is defined everywhere XState can run; this problem has come up in the past quite a few times.

I fixed it by using the Rollup replace plugin, but developers shouldn't need to do that. This problem also occurs when trying to use XState in the browser (e.g., via unpkg) without using xstate.web.js.

[Andarist]

I just find it a little bit ironic in general that the ecosystem has converged on using a tool that was designed to work with node (npm registry and CLIs interacting with it) and then started to complain about code distributes there to use node-specific conventions :shrug:

I understand the overall frustration with this but I'm a little bit torned between various goals here. Somewhat luckily we now have better ways of distributing code for various target environments, or when using different conditions (like development/production). Not that long time ago there was no standard for doing this - and even today most people probably won't be able to configure tooling correctly to pick up correct things. This is very nuanced and that's why I believe that using higher-level tooling that configured stuff on our behalf is very important.

We need to decide what are our defaults and priorities though, some of the goals include:

• good DX (in a form of additional warnings/errors, only available in the development)
• bundlesize (thanks to the former being hidden behind a development flag this one gets optimized)
• 0-config consumption

The problem with the current solution is that it violates first two. With the shape of this PR we regress on the second goal.

Take a look at what happens with a simple test:

// input
const IS_PRODUCTION =
  typeof process !== 'undefined'
    ? process.env.NODE_ENV === 'production'
    : false;

export default function () {
  if (IS_PRODUCTION) {
    console.log('prod');
  } else {
    console.log('dev');
  }
}

// terser output
const o="undefined"!=typeof process&&"production"===process.env.NODE_ENV;export default function(){o?console.log("prod"):console.log("dev")}

As we might see - the dev-only code has stayed untouched and is still present in the output. This is of no surprise, if we consider the type of IS_PRODUCTION it's a boolean here. A static tool can't evaluate the condition because it doesn't have access to the runtime information.

We might think that at least this will get fixed when one is using a "replacement" plugin. Take a look at such a situation then:

// input
const IS_PRODUCTION =
  typeof process !== 'undefined' ? 'production' === 'production' : false;

export default function () {
  if (IS_PRODUCTION) {
    console.log('prod');
  } else {
    console.log('dev');
  }
}

// output
const o="undefined"!=typeof process;export default function(){o?console.log("prod"):console.log("dev")}

Unfortunately, not much of a change here - the dev-only code is still here. Why is that? Well, the type of the IS_PRODUCTION didn't change here - it's still a boolean.

We can try to flip the situation and assume that we default to the production code (the previous version uses false as the default for IS_PRODUCTION):

// input
const IS_PRODUCTION =
  typeof process !== 'undefined' ? 'production' === 'production' : true;

export default function () {
  if (IS_PRODUCTION) {
    console.log('prod');
  } else {
    console.log('dev');
  }
}

// output
export default function(){console.log("prod")}

Ka-ching 💰 ! We have it! Well... do we? If we consult this with our overall goals then we might see that we've fixed the bundlesize problem but we've regressed on the DX part (by default, it can be gained back when using a bundler). It really is a Fast Good Cheap situation that we are having here.

Don't take me wrong - I'm open for adjusting this but I would like to make sure that we are making a well-informed decision here, taking all of that into consideration.

Nowadays we could remove process.env.NODE_ENV stuff from the distributes files entirely, thanks to package.json#exports. Unfortunately, in v4 we kinda allow accessing stuff from xstate/lib and xstate/es and this becomes more tricky than it should be and I'm somewhat afraid of making such changes in v4. And even with that in place - the webpack's recommendation is to default to the production code and make dev code opt-in. If we apply that we end up with the last situation that I've depicted - no improved DX by default. Fortunately, I think that most of modern, popular, bundling "wrappers" (like Vite, Snowpack, webpack 5, etc) know how to pick development/production code based on package.json#exports. So I guess that those who are hand-crafting their configs would just need to know how to opt into the development code on their own.

[davidkpiano]

@Andarist Should we close this or go with your solution (which I committed)?

[Andarist]

superseded by https://github.com/statelyai/xstate/pull/4016