Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Export * should allow named export overrides #1714

Closed
lexanth opened this issue May 19, 2021 · 6 comments · Fixed by #1909
Closed

Export * should allow named export overrides #1714

lexanth opened this issue May 19, 2021 · 6 comments · Fixed by #1909
Assignees
Labels
Milestone

Comments

@lexanth
Copy link

lexanth commented May 19, 2021

Describe the bug
When you do export * from 'x' and override something in * with a named export, according to the spec the named export should override the version from *. This pattern is recommended in e.g. https://testing-library.com/docs/react-testing-library/setup

Input code

import { customRender } from './customRender';

// re-export everything
export * from '@testing-library/react';

// override render method
export { customRender as render };

Config

{
  "sourceMaps": true,
  "jsc": {
    "parser": {
      "syntax": "typescript",
      "tsx": true,
      "dynamicImport": true
    },
    "transform": {
      "react": {
        "runtime": "automatic",
        "refresh": true
      },
      "hidden": {
        "jest": true
      }
    }
  },
  "module": {
    "type": "commonjs"
  },
  "env": {
    "coreJs": 3,
    "mode": "usage"
  }
}

Expected behavior
The transpilation should respect the behaviour that the named exports can override the wildcard exports.
e.g. Babel REPL generates the _exportNames and then excludes those when defining the wildcard export.

Other refs:

Version
The version of @swc/core: 1.2.55

Additional context
Workaround is to not depend on this behaviour

import * as rtl from '@testing-library/react';
import { customRender } from './customRender';

module.exports = {
  ...rtl,
  userEvent,
  render: customRender,
};
@lexanth lexanth added the C-bug label May 19, 2021
@kdy1 kdy1 modified the milestones: v1.2.58, v1.2.59 May 19, 2021
@atsikov
Copy link

atsikov commented May 24, 2021

Another case is that star exports should not override named exports
Such module should not export render from testing-library

export { customRender as render } from './customRender';

// re-export everything, but keep named 'render' exported above
export * from '@testing-library/react';

@kdy1 kdy1 modified the milestones: v1.2.59, v1.2.60 May 30, 2021
@kdy1 kdy1 modified the milestones: v1.2.60, v1.2.61 Jun 7, 2021
@kdy1 kdy1 modified the milestones: v1.2.61, v1.2.62 Jun 16, 2021
@kdy1 kdy1 modified the milestones: v1.2.62, v1.2.63 Jun 27, 2021
@kdy1 kdy1 modified the milestones: v1.2.63, v1.2.64 Jul 5, 2021
kdy1 added a commit to kdy1/swc that referenced this issue Jul 10, 2021
kdy1 added a commit to kdy1/swc that referenced this issue Jul 10, 2021
@kdy1 kdy1 self-assigned this Jul 10, 2021
kdy1 added a commit that referenced this issue Jul 10, 2021
swc_ecma_transforms_compat:
 - `regenerator`: Use es6 import while folding module. (#1641)
 - `typeof_symbol`: Handle `undefined` specially. (#1843)
 - `regenerator`: Do not create useless codes. (#1687)
 - `typeof_symbol`: Migrate to `VisitMut`.

swc_ecma_transforms_module:
 - Add `import_hoister`.
 - Improve import analyzer. (#1682)
 - Allow overriding `export *` wth named exports. (#1714)

swc_ecma_transforms_testing:
 - Add a hack for `regenerator-runtime`.

swc:
 - Run import analyzer ahead of time. (#1682)

misc:
 - Downgrade rustc to the version rust-analyzer supports.
@lexanth
Copy link
Author

lexanth commented Aug 12, 2021

@kdy1 This isn't quite fixed.

The code as currently output still gives a runtime error if the re-export is different from the initial export (which is the case in the example above - the aim is to override render with our custom version).

In your test (https://github.com/swc-project/swc/pull/1909/files#diff-b558773d8ee3e958908d1bdf859f470eabcd8ff82aee06841982ddcdcb6b11a2) it only checks if there is an existing export of that name with the same thing being exported (if (key in exports && exports[key] === _react[key]) return;).

In the babel output (REPL), it also tracks the list of named exports to not override

var _exportNames = {
  render: true
};

Object.keys(_react).forEach(function (key) {
  if (key === "default" || key === "__esModule") return;
  if (Object.prototype.hasOwnProperty.call(_exportNames, key)) return; // <-- This check is missing in the swc output
  if (key in exports && exports[key] === _react[key]) return;
  Object.defineProperty(exports, key, {
    enumerable: true,
    get: function () {
      return _react[key];
    }
  });
});

Without this check, at runtime you get an error from not being able to redefine the property (TypeError: Cannot redefine property: render)

@lexanth
Copy link
Author

lexanth commented Aug 18, 2021

@kdy1 Can you reopen this, or would you like a new issue?

@kdy1
Copy link
Member

kdy1 commented Aug 18, 2021

I prefer a new issue, would you open one?

@lexanth
Copy link
Author

lexanth commented Aug 18, 2021

Raised #2101 for this

@swc-bot
Copy link
Collaborator

swc-bot commented Oct 22, 2022

This closed issue has been automatically locked because it had no new activity for a month. If you are running into a similar issue, please create a new issue with the steps to reproduce. Thank you.

@swc-project swc-project locked as resolved and limited conversation to collaborators Oct 22, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Development

Successfully merging a pull request may close this issue.

4 participants