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

NCP resolves before file copy is completed #117

Closed
escottalexander opened this issue Sep 12, 2024 · 1 comment
Closed

NCP resolves before file copy is completed #117

escottalexander opened this issue Sep 12, 2024 · 1 comment

Comments

@escottalexander
Copy link

Just to clarify, I have not experienced any issues while creating extensions. This is just to raise awareness of an issue I encountered when using NCP in a similar way as the extension system. You are probably avoiding the actual issue because of the template processing that happens between the initial copying of files (with NCP) and the removal of the tempDir.

For me, I was removing the temporary directory right after NCP resolved and it was leaving half the files uncopied or empty. While trying to understand why this was happening I found an old issue that still exists with NCP: AvianFlu/ncp#127 Stll open and unresolved. At the end of the issue you see where facebook/react was able to sidestep the issue in their codebase by adding a 10ms timeout before resolving. Hacky but it apparently worked for them.

I was able to fix my problem with their same hacky solution:

const copy = (source: string, destination: string, options?: ncp.Options) => new Promise((resolve, reject) => {
    ncp(source, destination, options || {}, (err) => {
        if (err) {
            reject(err);
        } else {
            setTimeout(resolve, 10);
        }
    });
});

Again, no issues have been observed with the extension system as it is but I just wanted to make you aware of this potential issue when using NCP. Feel free to close this issue whenever you like.

@rin-st
Copy link
Member

rin-st commented Sep 13, 2024

thanks for reporting this! I think we won't change something for now since everything works as expected. But let's keep the issue open for some time so all maintainers can read it

@rin-st rin-st closed this as not planned Won't fix, can't repro, duplicate, stale Dec 20, 2024
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