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

TypeError when accessing exported member with "circular" dependency on barrel file #96

Open
wadjoh opened this issue Jan 2, 2024 · 7 comments
Labels
help wanted Extra attention is needed

Comments

@wadjoh
Copy link
Contributor

wadjoh commented Jan 2, 2024

Issue

Note that module.type in the swc config is set to commonjs for this issue.

When a member (in this case, a TS enum) is re-exported in a barrel file and a sub-dependency of the barrel file imports that enum through the barrel file and tries to access it, a TypeError is thrown:

TypeError: Cannot read properties of undefined (reading 'A')

This is due to how swc_mut_cjs_exports groups all the require calls at the top of the transpiled file and then adds the keys to the exports object after all of those require.

// task.js
var _enumDep = /*#__PURE__*/ _interop_require_wildcard(require("./enumDep"));
var _anotherDep = /*#__PURE__*/ _interop_require_wildcard(require("./anotherDep"));

/* ...helper functions... */

Object.keys(_enumDep).forEach(function(key) {
    if (key === "default" || key === "__esModule") return;
    if (Object.prototype.hasOwnProperty.call(exports, key)) return;
    Object.defineProperty(exports, key, {
        enumerable: true,
        get: function get() {
            return _enumDep[key];
        },
        configurable: true
    });
});
Object.keys(_anotherDep).forEach(function(key) {
    if (key === "default" || key === "__esModule") return;
    if (Object.prototype.hasOwnProperty.call(exports, key)) return;
    Object.defineProperty(exports, key, {
        enumerable: true,
        get: function get() {
            return _anotherDep[key];
        },
        configurable: true
    });
});

./enumDep exports SomeEnum, and ./anotherDep eventually imports SomeEnum from the task.js barrel file. But since the keys for ./enumDep haven't been added to exports by the time ./anotherDep is required, the SomeEnum ends up being undefined.

Reproduction

I've set up a small repo with the reproduction of this and the README has info on the structure:
https://github.com/wadjoh/swc_mut_cjs_exports_debug_ts_enum

This is the structure of the TS code that causes an error when the transpiled code is executed:
bug_explanation

Potential Fix

When swc_mut_cjs_exports transpiles star exports, this issue can be avoided by adding the keys to the exports object immediately after the associated require call:

// task.js

// moved helper function definitions to the top
/* ...helper functions... */

// `exports` is populated with module keys immediately after the module's `require` call
var _enumDep = /*#__PURE__*/ _interop_require_wildcard(require("./enumDep"));
Object.keys(_enumDep).forEach(function(key) {
    if (key === "default" || key === "__esModule") return;
    if (Object.prototype.hasOwnProperty.call(exports, key)) return;
    Object.defineProperty(exports, key, {
        enumerable: true,
        get: function get() {
            return _enumDep[key];
        },
        configurable: true
    });
});

var _anotherDep = /*#__PURE__*/ _interop_require_wildcard(require("./anotherDep"));
Object.keys(_anotherDep).forEach(function(key) {
    if (key === "default" || key === "__esModule") return;
    if (Object.prototype.hasOwnProperty.call(exports, key)) return;
    Object.defineProperty(exports, key, {
        enumerable: true,
        get: function get() {
            return _anotherDep[key];
        },
        configurable: true
    });
});

I have manually modified output code in the linked reproduction repo that works fine when executed: https://github.com/wadjoh/swc_mut_cjs_exports_debug_ts_enum/blob/main/results/fixed_output_with_plugin/task.js

@wadjoh
Copy link
Contributor Author

wadjoh commented Jan 2, 2024

@magic-akari I'm a newbie when it comes to Rust, but if you can point me in the right direction, I'd be happy to try and implement a fix for this myself.

@magic-akari
Copy link
Owner

You are right. This issue is complex.
Indeed, we should perform reexports immediately after calling require.

In fact, this plugin only handles export statements, leaving all import statements untouched in their original positions.

Regarding the following code:

export * from "a";
export * from "b";

It will be transformed into:

import * as _a from "a";
Object.keys(_a).forEach(function(key) {
    if (key === "default" || key === "__esModule") return;
    if (Object.prototype.hasOwnProperty.call(exports, key)) return;
    Object.defineProperty(exports, key, {
        enumerable: true,
        get: function get() {
            return _a[key];
        },
        configurable: true
    });
});
import * as _b from "b";
Object.keys(_b).forEach(function(key) {
    if (key === "default" || key === "__esModule") return;
    if (Object.prototype.hasOwnProperty.call(exports, key)) return;
    Object.defineProperty(exports, key, {
        enumerable: true,
        get: function get() {
            return _b[key];
        },
        configurable: true
    });
});

The reason for this approach is to account for other import statements, ensuring that the order of import statements remains unchanged.

However, import statements are hoisted, so the final result will show all import statements at the beginning, which means the translated require statements will be at the beginning as well.

@wadjoh
Copy link
Contributor Author

wadjoh commented Jan 3, 2024

That's what I was afraid of when I was looking through the code earlier.
It seems this plugin will output code in the correct order, but with import statements. Then swc comes along and hoists up the import statements before transforming them into require calls.

Is that right? If so, it sounds like maybe it's not something that could be fixed by this plugin?

@magic-akari
Copy link
Owner

I'm considering a new approach.
Could we simply add a line at the end of the file to change the mutability of exports?

For example:

module.exports = magicCopyAndMakeMutable(exports);

@wadjoh
Copy link
Contributor Author

wadjoh commented Jan 3, 2024

So the exports won't be mutable until the whole file has been executed? I'm not too familiar with the details of Jest mocking, but would module.exports be mutable by the time Jest tries to mock members of the modules?

@magic-akari magic-akari added the help wanted Extra attention is needed label Jan 3, 2024
@wadjoh
Copy link
Contributor Author

wadjoh commented Jan 30, 2024

I tried playing around with the module.exports = magicCopyAndMakeMutable(exports); approach, but couldn't get it working. Jest would run the tests without errors, but jest.spyOn.mockImplementation would have no effect. Tried a couple of different ways, but always had the same result. Not sure if I was just doing something wrong or not.

Another approach I tried was adding mutableDefineProperty function at the top of the file that looks like:

function mutableDefineProperty(obj, prop, attributes) {
    return Object.defineProperty(
        obj,
        prop,
        Object.assign({}, {configurable: true, enumerable: true}, attributes)
    )
}

Then replaced all instances of:

Object.defineProperty(exports, /* rest of the params */);

with

mutableDefineProperty(exports, /* rest of the params */);

That got the Jest tests with jest.spyOn.mockImplementation working as expected.

Any thoughts on that approach?
I also wonder if we could just forego the whole extra method and just add configurable: true to the third parameter object of all calls to Object.defineProperty where the first parameter is exports.

@magic-akari
Copy link
Owner

You can refer to the workaround for Jest mentioned here: #103 (comment)
or the Jest documentation: https://jestjs.io/docs/configuration#setupfilesafterenv-array

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

2 participants