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

dynamic requiring issue #578

Open
Songkeys opened this issue Aug 20, 2020 · 7 comments
Open

dynamic requiring issue #578

Songkeys opened this issue Aug 20, 2020 · 7 comments

Comments

@Songkeys
Copy link
Contributor

Songkeys commented Aug 20, 2020

This PR seems to have fixed the dynamic requiring but actually it's not?

Cases are complex for ncc to understand, even the test file failed:

I take the test file to test, here is the output:

{

/***/ 347:
/***/ (function(__unusedmodule, __unusedexports, __webpack_require__) {

const reaction = (name) => {
    const res = require(name);
    res.name = name.split("/").pop();
    return res;
},reaction$$mod = (nres, name) => { // note the first argument is `nres` but not the expected `res`, which fails the file!!
    
    res.name = name.split("/").pop();
    return res;
};

const reactions = {
    repository: {
        publicized: reaction$$mod(__webpack_require__(711), "./dep"),
    },
};


/***/ }),

/***/ 711:
/***/ (function(module) {

module.exports = "dep";


/***/ })

/******/ }

And here are some other test I've done

// case 1: same case as above but simplified
const dynamicRequire = (path) => {
    const res = require(path);
    return res;
};

dynamicRequire("./dep"); // seems working but not (result as above)
// case 2: same case as above but without `const` the function
dynamicRequire = (path) => {
    const res = require(path);
    return res;
};

dynamicRequire("./dep"); // not works!
// case 3: same case as above but directly returning the required result
const dynamicRequire = (name) => {
    return require(name);
};

dynamicRequire("./dep"); // not works!
// case 4: same case as above but directly assigning the require function
dynamicRequire = require;
dynamicRequire("./dep"); // not works!
@fearphage
Copy link

I'm seeing the same issue. Is there a setting or anything to workaround this issue?

@Songkeys
Copy link
Contributor Author

@fearphage

ncc has an update on 0.24.0 trying to solve this problem better. You can have a try.

But it doesn't solve my cases yet.

@fearphage
Copy link

My use case is similar. I walk a directory and require those files dynamically. I had to manually include them for now to get ncc to work as expected.

@Songkeys
Copy link
Contributor Author

Songkeys commented Oct 21, 2020

@fearphage

How did you manually include them? I tried to walk the directory myself and ncc them separately meanwhile maintaining the directory structure. Not a good way but I can't find any other options.

edit: I see. You meant you changed your "dynamic" behavior to get it work? But I can't do that cuz I'll never know what files will be in that folder.

@fearphage
Copy link

You meant you changed your "dynamic" behavior to get it work? But I can't do that cuz I'll never know what files will be in that folder.

That's correct. I moved from walking the directory to manually typing the file names into the require for each file. That's the only way I was able to continue using ncc.

@roryabraham
Copy link

I think I'm running into a related issue using react-native-version in a CI/CD script meant to run on a remote runner. It looks like that library internally uses path.cwd. I think that's causing ncc to include the absolute filepath on my local machine in its output, which won't work on the remote runner.

@devthejo
Copy link

devthejo commented Apr 1, 2022

take a look here #74 (comment)

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

4 participants