Skip to content

fix windows infinite loop#6

Open
yazonnile wants to merge 1 commit into
callumacrae:masterfrom
yazonnile:master
Open

fix windows infinite loop#6
yazonnile wants to merge 1 commit into
callumacrae:masterfrom
yazonnile:master

Conversation

@yazonnile

Copy link
Copy Markdown

all in comment :)

@callumacrae

Copy link
Copy Markdown
Owner

I don't understand the comment: what is actually happening here? Or rather, what was happening before?

@yazonnile

yazonnile commented Sep 2, 2016

Copy link
Copy Markdown
Author

What i have

D:/node_modules ---> node_modules folder in the root
D:/some-folder/app/node_modules
D:/some-folder/app/index.js --> file that use find-node-modules

So when I run index.js -> that do-while become infinite on windows
When I remove D:/node_modules folder - all work just fine.

If i make something like this

do {
    modulesDir = findup(options.searchFor, { cwd: searchDir });

    if (modulesDir !== null) {
        modulesArray.push(formatPath(modulesDir, options));
    }

    console.log(modulesDir);
    console.log(searchDir);
} while (modulesDir && (searchDir = path.join(modulesDir, '../../')));

I will get

D:\_progects\app
D:\_progects\app\node_modules
D:\_progects\
D:\node_modules
D:\
D:\node_modules
D:\
D:\node_modules
///...and so on...

So, all I did - just check searchDir for root

@callumacrae

callumacrae commented Sep 2, 2016

Copy link
Copy Markdown
Owner

That's odd. Do you know if this fix will affect non-windows systems at all?

@yazonnile

yazonnile commented Sep 2, 2016

Copy link
Copy Markdown
Author

I tested it on win7 on three pc and on macbook
win7 break the loop on the commited if
macos break loop with null on the modulesDir

@callumacrae

Copy link
Copy Markdown
Owner

Cool. I'll have a proper look at this PR over the weekend, thank you!

@yazonnile

Copy link
Copy Markdown
Author

Welcome!

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

Successfully merging this pull request may close these issues.

2 participants