Error with zustand exports in parcel production builds

[nicobohne]

Summary

A new error occurred in production builds using [email protected] and parcel. This error does not occur in [email protected] and is probably related to #1531.

The following error is displayed in the console when serving a production build:

Uncaught TypeError: (0 , i.create) is not a function
    at app.js:3:22

Development server works fine.

This issue is similar to #1475.

Link to reproduction

https://github.com/nicobohne/zustand-parcel-reproduction
https://codesandbox.io/p/github/nicobohne/zustand-parcel-reproduction/main

Check List

Please do not ask questions in issues.

• I've already opened a discussion before opening this issue, or already discussed in other media.

Please include a minimal reproduction.

• I've added a link to a typescript playground or codesandbox with a minimal reproduction.

[dai-shi]

Can @barelyhuman help please?

[barelyhuman]

I’ll check this out 👍

[barelyhuman]

;tldr

build with parcel build --no-scope-hoist for now

longer version

so, this is actually a problem with Parcel where during optimization and variable hoisting it ends up creating invalid assigns to the original export.

The related issue
parcel-bundler/parcel#7781

also, you can fix this for now by using --no-scope-hoist flag on your build script and it should work (this was confirmed) and is also mentioned in parcel-bundler/parcel#7781 (comment)

As for if something can be done by zustand?
I'm not sure, I can fix the generated build code for parcel, there's a chance an older version of parcel could handle this, so I'll check that out

Here's a screenshot of how you could fix it manually in the generated build, you disable the overlapping assignments on the vanilla.js exports and index.js exports of zustand.

a.k.a disable them for both the exported and used functions which in practicality would be impossible to do manually for a project with a lot of these kinds of libraries.

[drmax24]

--no-scope-hoist

inflates our build as far as i can see. Is there a different way around?

i have this error with zustand 4.3.3 and parcel 2.8.3

[barelyhuman]

I understand, sadly that's the current solution.

We could add in additional patchwork in the builds but if the final user bundler changes it, then it's very hard for us to control what the output would be.

I'll link an experimental zustand build in a bit that you can try and let me know if that solves your problem

[drmax24]

I'm on it. Plz send! I'll check it at once! We happen to use parcel (I think this is a mistake to use it). But we stuck with it so need to resolve this somehow for a production build that should be reasonably sized...

[barelyhuman]

@drmax24

Scope hoisting is mostly useful, so I wouldn't say so but yeah, this isn't something that's a problem with just parcel or zustand. It's more because of how we have mixed exports (both default and named) from the same file and is hence causing these issues.

It's not a straightforward task for parcel to handle this and so I wouldn't blame parcel for it, it's more a mistake from our end.

This is why we've been recommending people to move into using named exports instead.

Anyway, to the problem at hand

here's the experimental fix

https://ci.codesandbox.io/status/barelyhuman/zustand/pr/13/builds/350389

You can go through the Local Install Instructions on that link and use your pkg manager to install that build.

Once done, please do confirm that it works for you

Edit:
This is from my fork of zustand and while usable since it's fully in sync with the current latest, I'd recommend watching this thread if we decide to move this build fix into the main repo

[drmax24]

Works fine now. Thank you very much!

I've heard several opinions that export default is bad. But can you explain a little bit why it is an antipattern in your opinion?..

[barelyhuman]

TLDR;
It's not a default export problem but a mixed export problem.

The point is to not do both default and named exports from the same module.
If there's even one named export, then make them all as named exports.

And it's considered bad because you end up polluting the default export's prototype , which can cause
undefined behaviour because the this might change when polluting an object and the execution context might change when polluting a function's prototype

Longer Explanation

There's quite a few explanantions for this but our main reason was to keep the imports aesthetic and that backfired later due to introduction of ESM and we already had an API that was solidified by then.

The ES spec has export and export default which is a named export but the name is default , so in case it's being used by another bundler compatible with the ES Spec it's all good and it'll pickup the needed export as needed.

Now, on the other hand the node's CommonJS require pattern works with exports and module.exports both of which are tied to each other. The conversion of the ES to CommonJS turns the following code like this

// module1.js
export const a = () => {};
export const b = () => {};
export default a;
// ↓ ↓ ↓ ↓ ↓ ↓ ↓ ↓
const a = () => {};
const b = () => {};

exports.a = a;
exports.b = b;
exports.default = a;

// module2.js
export const a = () => {};
export const b = () => {};
export const c = () => {};
// ↓ ↓ ↓ ↓ ↓ ↓ ↓ ↓
const a = () => {};
const b = () => {};
const c = () => {};

exports.a = a;
exports.b = b;
exports.c = c;

Each of these can now be imported in the following manner

import aliasedA, { a, b } from "module1.js";

import * as pkg from "module2.js";
const { a, b, c } = pkg;

// or in commonjs
const aliasedA = require("module1.js").default;
const { a, b } = require("module1.js");
const { a, b, c } = require("module2.js");

This isn't that harmful since all that's changed is the added .default in CommonJS environments

Though, most people are used to having a bundler or compiler of some kind and some of them replace exports.default to modules.exports when converting to CJS if there's only one export and that's a default export. Others, don't, and so you'll get the exports.default

As a user, you'd want that consistency across environments, since you wouldn't wanna spend time figuring out
how the import is supposedly different based on where you are using it

// TS with moduleInterop off
import * as pkgOne from "module1.js";
const { a, b } = pkgOne;
// and pkgOne itself is also `a`

import * as pkg from "module2.js";
const { a, b, c } = pkg;

// TS with moduleInterop on
import pkgOne, { a, b } from "module2.js";
import { a, b, c } from "module2.js";

// esm
import pkgOne, { a, b } from "module2.js";
import { a, b, c } from "module2.js";

// cjs
const pkgOne = require("module2.js").default;
const { a, b } = pkgOne;

const { a, b, c } = require("module2.js");

Though, you'd see zustand,jotai,valtio have tried to maintain a very specific import pattern no matter
where you use it.

CJS, ESM, JEST Sandbox, it doesn't matter who handles the dependency resolution, we want people to maintain the same code and this is where all the problems show up.

This is what the current export for index.js is in zustand.

exports.create = create;
exports.default = react;
exports.useStore = useStore;

and here's how you'd have to write it in different envs if we didn't patch the exports

// ESM
import create from "zustand";

// CJS
const create = require("zustand").default;

// JEST
const create = require("zustand").default;

but if you've seen the docs, you don't have to, because we patch the exports on build time to also
export the default as module.exports

exports.create = create;
exports.default = react;
exports.useStore = useStore;

(module.exports = (exports && exports.default) || {}),
  Object.assign(module.exports, exports);

Which tries to go through all exports and if there's a default export, map it to module.exports
this changes the requires to be like this

// ESM
import create from "zustand";

// CJS
const create = require("zustand");

// JEST
const create = require("zustand");

Aesthetic but we still have a problem, we're basically polluting the function scope of the default export.
This can cause this binding issues and also change the context of execution in certain cases.

Which is better avoided. Now if I had only named exports , then I don't have to worry about bundlers
or the resolution algorithm

// ESM / TS with or without moduleInterop
import { create, useStore } from "zustand";

// CJS
const { create, useStore } = require("zustand");

No more patching required, so I won't be polluting another function/export prototype, and neither do I have worry about that accidentally changing my execution context.

The build fix I just made is actually adding the pollution manually as code instead of a programmatic patch which parcel is able to understand and hoist properly, it's not good but we'll have to wait till we can get rid of the default export altogether.

[nicobohne]

Thank you!