ESM files deployed to NPM package contain import references to CommonJS sibbling JS files (missing `.module` suffix))

[danielweck]

https://unpkg.com/browse/valtio@1.0.1/index.module.js

import { snapshot, subscribe, getVersion } from './vanilla';
export * from './vanilla';

...should point to https://unpkg.com/browse/valtio@1.0.1/vanilla.module.js ...but instead point to https://unpkg.com/browse/valtio@1.0.1/vanilla.js

Same issue with https://unpkg.com/browse/valtio@1.0.1/utils.module.js

import { subscribe, snapshot, proxy } from './vanilla';

[danielweck]

FYI, to work around this problem, I use @rollup/plugin-replace in my build process with the following configuration:

{
  include: /.+valtio.+/,
  delimiters: ['', ''],
  "'./vanilla'": "'./vanilla.module'"
}

[danielweck]

Ah, actually, when I use Preact WMR's development server, vanilla.module.js imported by index.module.js (via import ... from 'valtio') is not equal to the one imported by utils.module.js (via import ... from 'valtio/utils'), causing the proxy to be corrupted! So my final fix is in fact to replace import ... from './vanilla' to import ... from 'valtio/vanilla' in both index.module.js and utils.module.js. Thanks to the package.json exports, this works fine.

[dai-shi]

Thanks for reporting. This is causing by #112. So, unpkg follows "module" in package.json but not "exports"...

I somehow thought *.module.js is better (maybe because of types, but we are now copying them), but probably esm/*.js would solve it.

[danielweck]

Thank you for your prompt reply. I look forward to testing the fix, once merged and published.

I am hitting an interesting Preact WMR "bug" (maybe a "feature", I'm not sure), where the export map is correctly used by the Rollup-based production build system, but at development time some module caching seems to be involved which results in breaking the proxy functionality:

This happens when I import devtools() from valtio/util, which in turn invokes subscribe() from ./vanilla:

https://github.com/pmndrs/valtio/blob/8e625551783f2e6364670faf59411fe2b926793d/src/utils.ts#L64 => https://github.com/pmndrs/valtio/blob/8e625551783f2e6364670faf59411fe2b926793d/src/vanilla.ts#L188-L192

...and when I import proxy() from valtio (but crucially: transitively via ./vanilla)

https://github.com/pmndrs/valtio/blob/8e625551783f2e6364670faf59411fe2b926793d/src/index.ts#L1 => https://github.com/pmndrs/valtio/blob/8e625551783f2e6364670faf59411fe2b926793d/src/vanilla.ts#L40

Because of how Preact WMR loads modules, it seems the two ./vanilla are not the same modules in memory. Consequently, the LISTENERS Symbol prop is somehow undefined:

https://github.com/pmndrs/valtio/blob/8e625551783f2e6364670faf59411fe2b926793d/src/vanilla.ts#L103-L105

subscribe() would normally reveal that proxyObject[LISTENERS] is falsy instead of a Set():

https://github.com/pmndrs/valtio/blob/8e625551783f2e6364670faf59411fe2b926793d/src/vanilla.ts#L193-L199

...but Preact WMR removes process.env.NODE_ENV in its preprocessing phase, so I discovered this bug further down the call chain, when this line of code crashed:

https://github.com/pmndrs/valtio/blob/8e625551783f2e6364670faf59411fe2b926793d/src/vanilla.ts#L213

So, my workaround is to replace ./vanilla imports in Valtio's node_modules dist (./index and ./utils) with valtio/vanilla. This totally fixes the bug, I assume because Preact WMR's development server module cache indexer takes into account the export map correctly with valtio/vanilla symbolic references, resolving to a common ./vanilla module.

I reported a similar issue with Twind (which has since been fixed), but I think the situation was slightly different because each "sub package" is declared in the top-level export map AND has its own package.json (subfolder). See:

https://unpkg.com/browse/twind@0.16.10/package.json

e.g.: https://unpkg.com/browse/twind@0.16.10/css/package.json

I will post an issue in Preact WMR's tracker once I have a repro pattern, but in the meantime I am going to CC the main developers, just in case they have an idea: @marvinhagemeister @developit and @JoviDeCroock

[dai-shi]

So, my workaround is to replace ./vanilla imports in Valtio's node_modules dist (./index and ./utils) with valtio/vanilla.

This actually makes sense, but then it doesn't solve the original unpkg issue, does it? maybe it does. I wish we had a way to check it without publishing it.

[danielweck]

I think relative import references are a big no-no from one "sub package" to another (hypothetical example: ../vanilla/index.mjs from ./esm/utils/index.mjs), WHEN each "sub package" actually defines its own package.json in its own subfolder (e.g. valtio/core, valtio/vanilla, valtio/utils, valtio/react export map + consistent physical and logical organisation of "sub packages"). However valtio currently only defines an export map, without individual "sub packages". Perhaps this is the missing piece for better integration in the various modern systems like Preact WMR, SnowPack/SkyPack, Vite, etc.?

I'm not sure, but I should be able to try out the theory by hacking node_modules/valtio directly in my Preact WMR project (i.e. I can add package.json for each "sub package", try different export maps, and tweak Valtio's ESM import statements). I don't have the bandwidth to experiment right now, but I will get to it :)

[danielweck]

it doesn't solve the original unpkg issue

I'm not sure what you mean by that. I provided Unpkg links in my comments purely to display the raw contents of Valtio dist files, just as they appear in local node_modules. Unpkg just mirrors the NPM registry, unlike SkyPack etc. which are CDNs that feature "smart" ESM support and transparent multi-package bundling.

[dai-shi]

I'm not sure what you mean by that.

Oops, I was confused with something. So, the original issue and the behavior with Preact WMR are different things.

The original issue should be resolved by #139.

The behavior you mentioned about Preact WMR sounds conflicting with #112 / #110. (but, it's not certain if #112 really fixed #110.) We may need some help on this.

Let me merge #139 as it should be a good fix.

[danielweck]

yes I agree with your analysis. one step at a time :) 👍

[danielweck]

I can confirm that strictly-speaking, the fix works, thank you :)

However, the module caching problem persists with Preact WMR (as expected), and I wonder whether this affects other "modern" build systems as well.

So, I continue to patch Valtio via my package.json postinstall script, replacing relative import references to absolute ones, as per Valtio's export map (e.g. ./vanilla to valtio/vanilla). Otherwise, this:

[dai-shi]

@danielweck You are absolutely right. #112 was a wrong fix. It didn't fix #110. I don't know how to fix #110, but let me revert #112.