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 from should be passed through _interopRequireDefault #757

Open
forivall opened this issue Oct 13, 2022 · 2 comments
Open

export from should be passed through _interopRequireDefault #757

forivall opened this issue Oct 13, 2022 · 2 comments

Comments

@forivall
Copy link
Contributor

forivall commented Oct 13, 2022

The two source files should generate equivalent imports transformation:

Sandbox

import foo from 'bar'
export { foo }

Sandbox

export { default as foo } from 'bar'

The output is identical with babel, and typescript uses its __importDefault in a different order. Sucrase only applies the _interopRequireDefault in the first example.

IMO, the sucrase transformed code should be as follows:

"use strict";Object.defineProperty(exports, "__esModule", {value: true}); function _interopRequireDefault(obj) { return obj && obj.__esModule ? obj : { default: obj }; } function _createNamedExportFrom(obj, localName, importedName) { Object.defineProperty(exports, localName, {enumerable: true, configurable: true, get: () => obj[importedName]}); }
var _bar = require('bar'); var _bar2 = _interopRequireDefault(_bar); _createNamedExportFrom(_bar2, 'foo', 'default');

(unless enableLegacyTypeScriptModuleInterop is enabled, obvi)

@alangpierce
Copy link
Owner

Hi @forivall , thanks for reporting! I agree that this is a bug. To be clear, the impact is that the re-export won't work correctly if "bar" is a regular CJS module, e.g. if it has a line like module.exports = "Hello";. I'm a bit busy at the moment, but may be able to find some time for it, or a PR would be welcome. There was already a similar fix in #619 (which is why the two-line version works), so the fix would probably be something similar.

(A separate but related issue is whether to use live bindings vs a plain export assignment for the two-line version. That came up in #715 (comment) , and I have some hesitation with switching the behavior due to possible backcompat issues, though it might be fine.)

@forivall
Copy link
Contributor Author

No worries on timing. The issue doesnt impact anything at the moment, I just noticed when making notes of all of the quirks of sucrase that I need to be aware of.

As far as the transitive binding, I think the export {...} from should use live bindings, since that's what tsc does (even for legacy interop), but tsc doesn't do live bindings for the 2 line export.

Ideally, there should be flags for this, although, that does introduce maintenance burden.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants