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

Wrong module returned when both dir and file has same name #211

Closed
SimenB opened this issue Feb 5, 2020 · 1 comment · Fixed by #212
Closed

Wrong module returned when both dir and file has same name #211

SimenB opened this issue Feb 5, 2020 · 1 comment · Fixed by #212

Comments

@SimenB
Copy link
Contributor

SimenB commented Feb 5, 2020

Pretty much the same as #51, but from within the directory this time. See https://github.com/SimenB/resolve-file-and-dir-name-clash-bug

It works correctly for ./ but not ..

Swapping

var m = loadAsFileSync(res) || loadAsDirectorySync(res);

to

var m = loadAsDirectorySync(res) || loadAsFileSync(res); 

"fixes" the issue, but breaks the test added for #51.

Here are two failing unit tests (for sync and async):

diff --git i/test/resolver.js w/test/resolver.js
index 5df8e1d..2b8475a 100644
--- i/test/resolver.js
+++ w/test/resolver.js
@@ -315,6 +315,22 @@ test('#52 - incorrectly resolves module-paths like "./someFolder/" when there is
     });
 });
 
+test('#211 - incorrectly resolves module-paths like "." when from inside a folder with a sibling file of the same name', function (t) {
+    t.plan(2);
+
+    var dir = path.join(__dirname, 'resolver');
+
+    resolve('./', { basedir: path.join(dir, 'same_names/foo') }, function (err, res, pkg) {
+        if (err) t.fail(err);
+        t.equal(res, path.join(dir, 'same_names/foo/index.js'));
+    });
+
+    resolve('.', { basedir: path.join(dir, 'same_names/foo') }, function (err, res, pkg) {
+        if (err) t.fail(err);
+        t.equal(res, path.join(dir, 'same_names/foo/index.js'));
+    });
+});
+
 test('async: #121 - treating an existing file as a dir when no basedir', function (t) {
     var testFile = path.basename(__filename);
 
diff --git i/test/resolver_sync.js w/test/resolver_sync.js
index aa8e256..0e2ab38 100644
--- i/test/resolver_sync.js
+++ w/test/resolver_sync.js
@@ -238,6 +238,20 @@ test('#52 - incorrectly resolves module-paths like "./someFolder/" when there is
     t.end();
 });
 
+test('#211 - incorrectly resolves module-paths like "." when from inside a folder with a sibling file of the same name', function (t) {
+    var dir = path.join(__dirname, 'resolver');
+
+    t.equal(
+        resolve.sync('./', { basedir: path.join(dir, 'same_names/foo') }),
+        path.join(dir, 'same_names/foo/index.js')
+    );
+    t.equal(
+        resolve.sync('.', { basedir: path.join(dir, 'same_names/foo') }),
+        path.join(dir, 'same_names/foo/index.js')
+    );
+    t.end();
+});
+
 test('sync: #121 - treating an existing file as a dir when no basedir', function (t) {
     var testFile = path.basename(__filename);
 

This was originally reported to Jest in jestjs/jest#8910 (Jest uses a forked implementation of resolve under the hood). You can look at my PR which fixes it in Jest. It stalled since I wasn't happy with the implementation, but maybe you can draw inspiration from it, I dunno 🙂 jestjs/jest#8912

@SimenB
Copy link
Contributor Author

SimenB commented Feb 5, 2020

Actually, copying my fix from the Jest PR makes the tests I added here pass (it also fixes the test in the reproduction repository) - I'll open up a PR

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

Successfully merging a pull request may close this issue.

1 participant