parcel icon indicating copy to clipboard operation
parcel copied to clipboard

Checking optional exports with truthyness gives `does not export`

Open wherget opened this issue 2 years ago β€’ 6 comments

πŸ› bug report

Some packages (emotion, material-ui) try to support multiple upstream library versions (react) by checking for exported symbols as follows:

import * as other from './other.js';

const maybeFoo = other['foo']; // expecting 'undefined' when export is missing
if (maybeFoo) { maybeFoo() }

Parcel (somewhat justifiably) errors out with

error: src/other.js does not export 'foo'

πŸ€” Expected Behavior

The libraries to compile with parcel.

😯 Current Behavior

e.g. (for preact 10.10.3 and material-ui 5.10.1)

@parcel/core: node_modules/preact/compat/dist/compat.module.js does not export 'useId'

  node_modules/@mui/utils/esm/useId.js:21:25
    20 | 
  > 21 | const maybeReactUseId = React['useId' + ''];
  >    |                         ^^^^^^^^^^^^^^^^^^^
    22 | /**
    23 |  *

See also: Failing Vercel repro.

πŸ’ Possible Solution

Guarding the import like

const maybeFoo = ('foo' in other) ? other['foo'] : undefined;

obviously produces the desired result. (Vercel repro)

Disabling scope hoisting (Vercel repro) also works around this issue, but is obviously not desirable for a production build.

Webpack (from https://github.com/webpack/webpack/issues/14814) has strictExportPresence as a toggle.

πŸ”¦ Context

Maybe someone with more insight into what the expected behaviour would be could chime in. I like that parcel detects this as a possible import problem, but OTOH I feel like expecting undefined for a missing (or, well, undefined) export isn't too unrealistic an expectation.

Possibly related: #7867 (which does not occur in my project (emotion 11.10, preact 10.10), so I'm submitting this as a different issue.)

At the end of the day, I just want my project to build. πŸ˜…

wherget avatar Aug 19 '22 18:08 wherget

another similar pattern I just came across (in https://github.com/ariakit/ariakit) is

const maybeFoo = typeof other["foo"] === "function" ? other["foo"] : undefined;

which seems redundant (as in, if it's not a function, it's probably undefined) to me?

wherget avatar Aug 19 '22 18:08 wherget

Guarding the import like obviously produces the desired result.

To explain what's going on here: For better tree shaking, Parcel treats

import * as other from './other.js';
console.log(other.foo);

as

import {foo}Β from './other.js';
console.log(foo);

This is only possible as long as other is used only for member expressions, so adding console.log(other) would prevent this optimization and therefore also prevents the does not export error (and similarly your "x" in other check)

A big reason why we want to support this even for computed member expressions (other["foo"]) is that it makes using CSS modules easier, where you would do styles["my-class"] which isn't possible with the other.foo syntax.

Possibly related: https://github.com/parcel-bundler/parcel/issues/7867

Yeah, that's the same problem.

mischnic avatar Aug 22 '22 12:08 mischnic

Oh yes, I see how this is important for optimizing css modules, even better that it works for computed expressions. While I personally think that "x" in other would be a sensible check to add upstream for these "dependency might not export symbol" situations, I might be among a minority with that sentiment.

Do you think it might be useful to check if the member expression is used in a truthyness or type check somewhere down the tree, i.e. where undefined might be a valid value? Possibly with a warning that such an access downgrades export checking. I mean, it's always hard to say what users might do in the real world, but my gut feeling is that those would infrequently occur on css modules.

Theoretically, you could even prune away pure calls resulting from such a check at compile time, as we'd know the export is missing. Right?

wherget avatar Aug 22 '22 14:08 wherget

Hi everyone! I have a workaround of this problem when you cannot modify the pattern, for example in a library.

I had the same issue with Jotai:

🚨 Build failed.

@parcel/core: /node_modules/preact/compat/dist/compat.module.js does not export 'use'

  /node_modules/jotai/react.js:26:11
    25 | };
  > 26 | var use = ReactExports.use || function (promise) {
  >    |           ^^^^^^^^^^^^^^^^
    27 |   if (promise.status === 'pending') {
    28 |     throw promise;

error Command failed with exit code 1.

So, I take advantage of file alias system that parcel uses, so an alias entry to file needs to be created:

package.json:

  "..."
    "alias": {
        "react": "./preact.js",
        "react-dom": "preact/compat",
        "..."
    }

and the file has this content:

// preact.js
import preact from "preact/compat"
export * from "preact/compat"
export const use = false // I use false here to define the symbol AND exit the if-else or ||  pattern
export default preact

That's all. Parcel compiles without the need to deactivate scope hoisting!

sytabaresa avatar Jun 08 '23 16:06 sytabaresa

@sytabaresa thanks for the update! Is there anything I can do with regular react to get react-select running using the alias trick? I already tried a bit but failed quite hard with nothing working at all.

bartrail avatar Jun 09 '23 16:06 bartrail

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 14 days if no further activity occurs.

github-actions[bot] avatar Jun 05 '24 12:06 github-actions[bot]