Nested `default` exports with using dynamic import #813

antfu · 2021-12-21T18:09:25Z

@testing-library/user-event version: 13.5.0

Testing Framework and version: Vitest v0.0.106
DOM Environment: None

Relevant code or config

When using Node's native dynamic import, nested default is exported, causing the resolving to fail.
Related vitest-dev/vitest#243

➜ node            
Welcome to Node.js v16.13.0.
Type ".help" for more information.
> const a = await import('@testing-library/user-event')
undefined
> a
[Module: null prototype] {
  __esModule: true,
  default: {
    default: {
      click: [Function: click],
      dblClick: [Function: dblClick],
      type: [Function: type],
      clear: [Function: clear],
      tab: [Function: tab],
      hover: [Function: hover],
      unhover: [Function: unhover],
      upload: [Function: upload],
      selectOptions: [Function: bound selectOptionsBase],
      deselectOptions: [Function: bound selectOptionsBase],
      paste: [Function: paste],
      keyboard: [Function: keyboard]
    },
    specialChars: [Getter]
  },
  specialChars: {
    arrowLeft: '{arrowleft}',
    arrowRight: '{arrowright}',
    arrowDown: '{arrowdown}',
    arrowUp: '{arrowup}',
    enter: '{enter}',
    escape: '{esc}',
    delete: '{del}',
    backspace: '{backspace}',
    home: '{home}',
    end: '{end}',
    selectAll: '{selectall}',
    space: '{space}',
    whitespace: ' ',
    pageUp: '{pageUp}',
    pageDown: '{pageDown}'
  }
}

The text was updated successfully, but these errors were encountered:

ph-fritsche · 2021-12-21T20:52:30Z

Please have a look at #757 (comment)
Could you also check if you have the same problem with v14.0.0-beta?

sheremet-va · 2021-12-22T09:42:29Z

This is pure ESM Node without any transpilers: https://stackblitz.com/edit/node-mucfca?file=index.js

I'm not sure if exports.default is correct representation of default export. Isn't it module.exports? Maybe you need to update your bundling proccess?

I think bundlers tried to support this way of exporting default, but official implementation doesn't. Babel has a lot of experimental things that didn't end up in Node/browsers and the whole point of using babel is to make it run everywhere 😄

ph-fritsche · 2021-12-22T15:07:14Z

exports is an alias for module.exports. https://stackblitz.com/edit/node-epmcis?file=index.js

You could set exports, but this breaks the named exports: https://stackblitz.com/edit/node-vv2bff?file=index.mjs

ESM differentiates default export from named exports. CJM does not.

This is how @babel and tsc both transpile ES modules with default export:

Object.defineProperty(exports, "__esModule", {
  value: true
});
Object.defineProperty(exports, "default", {
  enumerable: true,
  get: function () {
    return // ... value ...
  }
});
Object.defineProperty(exports, "someNamedExport", {
  enumerable: true,
  get: function () {
    return // ... value ...
  }
});

It looks like the consuming environment is lacking interopRequireDefault polyfill.

function _interopRequireDefault(obj) { return obj && obj.__esModule ? obj : { default: obj }; }

I'm not sure about the best export moving forward.
We could add an ESM export like I do e.g. here: https://unpkg.com/browse/[email protected]/ .
The resulting package works out-of-the-box in node per require and in tsc by any means. But at the moment this still requires to use a bundler before using the package in a browser or --es-module-specifier-resolution=node flag to import it in node packages with type: "module". I don't like the idea to replace the imports in source with semantically incorrect references to files that don't exist (yet). And I'm not convinced yet that running e.g. https://github.com/antongolub/tsc-esm-fix as a default really solves all problems.

sheremet-va · 2021-12-22T15:30:09Z

I don't think Node should watch at the __esModule property since it was invented by babel at the early stages of ESM just to differentiate between CJS and ESM, and used only by compilers.

I would prefer shipping both CJS and ESM and specifying "module": "./dist/index.esm.mjs" (or something like that) in package.json.
It doesn't require any additional flags on user's end as far as I'm concerned.
That way we can use this package without any transpiling, using native ESM.

Also what you provided does not look like what user-event is shipping (https://unpkg.com/browse/@testing-library/[email protected]/dist/index.js)

By the way I should point out that named exports are working fine, as you can see in @antfu issue. Only default one is kinda fishy

ph-fritsche · 2021-12-22T16:02:38Z

Also what you provided does not look like what user-event is shipping (https://unpkg.com/browse/@testing-library/[email protected]/dist/index.js)

I know, but it's equivalent here.

I don't think Node should watch at the __esModule property since it was invented by babel at the early stages of ESM just to differentiate between CJS and ESM, and used only by compilers.

There is no default export in CJS. That's what __esModule is for. To provide the same functionality for consuming projects as if the export was in ESM.

I would prefer shipping both CJS and ESM and specifying "module": "./dist/index.esm.mjs" (or something like that) in package.json.
It doesn't require any additional flags on user's end as far as I'm concerned.
That way we can use this package without any transpiling, using native ESM.

Sadly it seems not to be that simple. There is no way CJS and ESM can be consumed the same way without some magic like __esModule. At least this is the case for the default export.
Also at the moment there is no way to output valid ESM via tsc unless you reference non-existing files in the source. So this would need to be solved by a bundler, a flag or running a fixup on the build like mentioned above.

sheremet-va · 2021-12-22T18:33:26Z

Sadly it seems not to be that simple. There is no way CJS and ESM can be consumed the same way without some magic like __esModule. At least this is the case for the default export.

What do you mean? If you bundle cjs/esm in different files, you can set something like this in package.json:

  "exports": {
    "import": "./main-module.js",
    "require": "./main-require.cjs"
  },

Also at the moment there is no way to output valid ESM via tsc unless you reference non-existing files in the source

Why do you need to use tsc? Is it a requirement? I suggest using tsup. You can specify format as an array of 'cjs and 'esm'.

Maybe there are other ways. I know @antfu helped other packages fix similar issues while working on Vite. Maybe he has something to say?

ph-fritsche · 2021-12-22T19:05:52Z

What do you mean? If you bundle cjs/esm in different files, you can set something like this in package.json:
  "exports": {
    "import": "./main-module.js",
    "require": "./main-require.cjs"
  },

If you bothered to read my posts and followed the links...

Also at the moment there is no way to output valid ESM via tsc unless you reference non-existing files in the source

Why do you need to use tsc? Is it a requirement? I suggest using tsup. You can specify format as an array of 'cjs and 'esm'.

Yes, a bundler is an option as mentioned before.
No, using tsc is not a requirement. Currently the build is done via kcd-scripts per Babel and Rollup.

sheremet-va · 2021-12-22T19:17:42Z

If you bothered to read my posts and followed the links...

I did. And they never explain why it can't be done. I see why you can't put them in one file, but there is nothing that stops you provide different files for different formats. I even gave you a link to Nodejs documentation that tells you how it can be done and provided a way to bundle it.

Also you are being rude. My only goal is to help you be compatible with native Node ESM implementation and not to fight.

ph-fritsche · 2021-12-22T19:47:59Z

I'm sorry if I came off as rude. It's just that this isn't that simple as you're making it out to be. I guess otherwise there would be a solution that everybody would be using. I've read quite a few posts and asked a few people about this when updating the script used to build the package with dual export mentioned above.

CJS and ESM are not interchangeable. At least not for the default export which does not exist in CJS. Therefore __esModule has to exist so that they could be consumed the same way.

Also there is a very long discussion in TypeScript repo over multiple issues why they won't manipulate any import path. I tend to disagree regarding their stance that it also should not be possible per a compiler flag, but they do have a point in the possible downfalls of it which also apply here.

Also I know a dual export can be done for some environments. But this is exactly the problem: No solution so far seems to be working for all environments. This is due to the fact that how imports are resolved is subject to the environment. There are mechanisms described e.g. in the NodeJS docs how to make this work e.g. per exports map, but they are not supported by all environments.
Any approach I've seen so far solves one issue by creating another. That's why I'm still not sure what the best way to move forward is here.

sheremet-va · 2021-12-22T20:02:45Z

Also I know a dual export can be done for some environments. But this is exactly the problem: No solution so far seems to be working for all environments.

What environments are you refering to? Where having two files with exports like this won't work? And how does it affect userbase?

  "main": "./dist/index.cjs",
  "exports": {
    "import": "./dist/index.js",
    "require": "./dist/index.cjs"
  },

If you require it in Node, it will return from ./dist/index.cjs. If Node doesnt support exports (<12), it will get from ./dist/index.cjs, where it suppose to.
If you import it in Node, it will return from ./dist/index.js. And no one uses import in Node < 12.

Also all bundlers support exports field now, if you rely on them, and not on native Nodejs. I thought it was a standard practic to be honest, while the ecosystem is moving gradually from CJS to ESM.

That's what unplugin-auto-import does, for example

antfu · 2021-12-22T20:11:54Z

I think the ideal way is indeed to ship dual formats (CJS & ESM), so that Node can understand and import it correctly based on the user environment. Without an additional layer from other tools to do the interop. It's the standard, and I believe it will benefit in the long run.

While in our particular cases on Vitest, interpretation is acceptable and it's also what we are already doing. The only blocker is the nesting. I would expect the correct format to be:

{
  __esModule: true,
  default: {
    click: [Function: click],
    dblClick: [Function: dblClick],
    type: [Function: type],
    clear: [Function: clear],
    tab: [Function: tab],
    hover: [Function: hover],
    unhover: [Function: unhover],
    upload: [Function: upload],
    selectOptions: [Function: bound selectOptionsBase],
    deselectOptions: [Function: bound selectOptionsBase],
  }
}

ph-fritsche · 2021-12-23T10:27:28Z

What environments are you refering to? Where having two files with exports like this won't work? And how does it affect userbase?

I'm not sure if there are environments out there that would already break on main imports.
IIRC deep imports according to export map like described in the docs do break in older node versions (<=14 ???).
We might be able to decide that we don't support these here (which would imply a breaking change).
But we use deep imports from other packages which we currently share the build script with. So this either implies copying & maintaining those code snippets in this repo too, or we'd have to have different build scripts forever (or at least until node 14?? is faded out).
IIRC this only affects @testing-library/dom at the moment, but I'm not sure any of those options would be a great idea.

I'm not against a change here. A well written and researched PR would be welcome.
But to me it doesn't look that easy to accomplish compatibility here without risking breaking code for some of the >3m weekly downloads.
That's why I implemented dual exports in other packages while being skeptical if it's a good idea here.

It doesn't help if you throw in hunches how this might be improved if you leave it to me to research how these might affect some of our users.

ph-fritsche · 2021-12-23T10:54:59Z

I think the ideal way is indeed to ship dual formats (CJS & ESM), so that Node can understand and import it correctly based on the user environment. Without an additional layer from other tools to do the interop. It's the standard, and I believe it will benefit in the long run.

Agree.
An additional layer is needed as the import paths in our source don't have a universal translation to valid ESM as there is no universal module resolution strategy for the consuming side.
So we would need to add this additional layer to the build process.

While in our particular cases on Vitest, interpretation is acceptable and it's also what we are already doing. The only blocker is the nesting.

The problem seems to be that the library was designed to be consumed per default export while being exported as CJS which has no differentiation for default and named exports. Changing this would be a big breaking change.

I would expect the correct format to be:

{
  __esModule: true,
  default: {
    click: [Function: click],
    dblClick: [Function: dblClick],
    type: [Function: type],
    clear: [Function: clear],
    tab: [Function: tab],
    hover: [Function: hover],
    unhover: [Function: unhover],
    upload: [Function: upload],
    selectOptions: [Function: bound selectOptionsBase],
    deselectOptions: [Function: bound selectOptionsBase],
  }
}

That is what is seen in other environments that do support __esModule.

antfu · 2021-12-23T13:46:15Z

The problem is that we are not talking about "some environments", it's native Node. I would be surprised to see a node package that does not work on the node directly but require external tools to interop every single file to work (this make thing a lot less efficient). Interoperation for top-level import is understandable since the different expressive between CJS and ESM, but for nested modules, there is nothing users could do to interop them without hacks or bundling.

Thanks for following up on this thread. I am not forcing you to make changes. But as a maintainer of many packages, I would think is even breaking change is worth it to make things correct and future-proofing.

ph-fritsche · 2021-12-23T17:48:41Z

@antfu How would you address the following concerns?

There is no default export in CJS. So if we don't want to completely change the way users interact with this library, the interop is still needed for any consuming project that imports the CJS. Dropping CJS is not an option as long as we support node versions without stable ESM-support.
If we wan't to keep our source semantically correct, there is no universal translation of import paths for ESM. So if we don't bundle our export or run a fixup changing the import paths, the consuming projects would still need to use node with es-module-specifier-resolution flag or use a bundler/transpiler that supports node module resolution.
But if we do bundle or run a fixup we make assumptions about the consuming environment. Those assumptions might be correct, but we won't know until people complain about breaking instances.
If we use different entry points this requires us to use an export map for deep imports. This means deep imports will be blocked as long as we support older node versions. This might be feasible for this repo, but it is unclear if it would be feasible for other libs we share the build script with. So we'd have to diverge and stay that way at least for the foreseeable future.
If other libs agreed to not support deep imports, we'd have to copy&maintain solutions that are already tracked in other repos.

If we can find a solution that really solves/eliminates a problem, that would be great.
But there is already a lack of contributions and maintenance support for this repo.
If we diverge regarding the infrastructure, this increases the burden for other team members when they try to help out here.
And there is nobody else who will deal with the issues, should our changes break consuming projects. So any solution should be bullet-proof.

ph-fritsche · 2021-12-30T07:46:39Z

Resolved in v14.0.0-beta.4.
I hope the solution "just works" for everybody.

ph-fritsche · 2021-12-31T16:59:06Z

Sadly the solution does not "just work".
Using this export with Jest requires at least experimental https://nodejs.org/api/vm.html#vm_class_vm_module
Using this with ts-jest also requires:

    preset: 'ts-jest/presets/default-esm',
    globals: {
        'ts-jest': {
            useESM: true,
        },
    },

github-actions · 2021-12-31T17:39:14Z

🎉 This issue has been resolved in version 14.0.0-beta.5 🎉

The release is available on:

Your semantic-release bot 📦🚀

antfu mentioned this issue Dec 21, 2021

Testing Library User Event Error vitest-dev/vitest#243

Closed

6 tasks

ph-fritsche mentioned this issue Dec 29, 2021

fix!: export package as bundled ESM #816

Merged

1 task

ph-fritsche added this to the userEvent v14 milestone Dec 29, 2021

ph-fritsche linked a pull request Dec 29, 2021 that will close this issue

fix!: export package as bundled ESM #816

Merged

1 task

ph-fritsche closed this as completed Dec 30, 2021

ph-fritsche added the released on @beta label Dec 30, 2021

ph-fritsche reopened this Dec 31, 2021

ph-fritsche mentioned this issue Dec 31, 2021

fix: export types and commonjs bundle #821

Merged

1 task

ph-fritsche closed this as completed Dec 31, 2021

tada5hi mentioned this issue Jan 18, 2024

Bug: OptionsError: The database options could not be located/loaded. tada5hi/typeorm-extension#845

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Nested `default` exports with using dynamic import #813

Nested `default` exports with using dynamic import #813

antfu commented Dec 21, 2021

ph-fritsche commented Dec 21, 2021

sheremet-va commented Dec 22, 2021 •

edited

Loading

ph-fritsche commented Dec 22, 2021

sheremet-va commented Dec 22, 2021 •

edited

Loading

ph-fritsche commented Dec 22, 2021 •

edited

Loading

sheremet-va commented Dec 22, 2021 •

edited

Loading

ph-fritsche commented Dec 22, 2021

sheremet-va commented Dec 22, 2021 •

edited

Loading

ph-fritsche commented Dec 22, 2021

sheremet-va commented Dec 22, 2021 •

edited

Loading

antfu commented Dec 22, 2021 •

edited

Loading

ph-fritsche commented Dec 23, 2021

ph-fritsche commented Dec 23, 2021

antfu commented Dec 23, 2021 •

edited

Loading

ph-fritsche commented Dec 23, 2021

ph-fritsche commented Dec 30, 2021

ph-fritsche commented Dec 31, 2021

github-actions bot commented Dec 31, 2021

Nested default exports with using dynamic import #813

Nested default exports with using dynamic import #813

Comments

antfu commented Dec 21, 2021

ph-fritsche commented Dec 21, 2021

sheremet-va commented Dec 22, 2021 • edited Loading

ph-fritsche commented Dec 22, 2021

sheremet-va commented Dec 22, 2021 • edited Loading

ph-fritsche commented Dec 22, 2021 • edited Loading

sheremet-va commented Dec 22, 2021 • edited Loading

ph-fritsche commented Dec 22, 2021

sheremet-va commented Dec 22, 2021 • edited Loading

ph-fritsche commented Dec 22, 2021

sheremet-va commented Dec 22, 2021 • edited Loading

antfu commented Dec 22, 2021 • edited Loading

ph-fritsche commented Dec 23, 2021

ph-fritsche commented Dec 23, 2021

antfu commented Dec 23, 2021 • edited Loading

ph-fritsche commented Dec 23, 2021

ph-fritsche commented Dec 30, 2021

ph-fritsche commented Dec 31, 2021

github-actions bot commented Dec 31, 2021

Nested `default` exports with using dynamic import #813

Nested `default` exports with using dynamic import #813

sheremet-va commented Dec 22, 2021 •

edited

Loading

sheremet-va commented Dec 22, 2021 •

edited

Loading

ph-fritsche commented Dec 22, 2021 •

edited

Loading

sheremet-va commented Dec 22, 2021 •

edited

Loading

sheremet-va commented Dec 22, 2021 •

edited

Loading

sheremet-va commented Dec 22, 2021 •

edited

Loading

antfu commented Dec 22, 2021 •

edited

Loading

antfu commented Dec 23, 2021 •

edited

Loading