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

fix: . and ./ incorrectly converted #409

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

relign
Copy link

@relign relign commented Sep 15, 2020

. and ./ are specific paths, we can't transfrom them.
There are many projects(example) that use import xxx from '.' .
Example code:

// index.js
export var name = 'zhangsan'
// test.js
import { name } from '.'
console.log('name', name)

The directory structure:

├── src
│   ├── index.js
│   └── mock
│       ├── index.js
│       └── test.js

Babel config:

["module-resolver", {
            "root": ["./src"]
        }]

Babel Transform Output:

// test.js
import { name } from "./..";
console.log('name', name);

And it should transfrom code import { name } from './index', not import { name } from './..'

@fatfisz
Copy link
Contributor

fatfisz commented Sep 15, 2020

Please add some tests too 🙏

@fatfisz
Copy link
Contributor

fatfisz commented Sep 15, 2020

Also this should probably affect paths like foo/bar/., and not only single dots.

Now that I think of it, maybe adding an option that introduces this behavior instead of the default one would be the best (because it's non-standard compared to Node resolve algorithm).

@relign
Copy link
Author

relign commented Sep 16, 2020

Please add some tests too 🙏

Test Case have been supplemented

@relign
Copy link
Author

relign commented Sep 16, 2020

Also this should probably affect paths like foo/bar/., and not only single dots.

Now that I think of it, maybe adding an option that introduces this behavior instead of the default one would be the best (because it's non-standard compared to Node resolve algorithm).

like foo/bar/. , it is no problem after test.
I agree with you that this is not a standard usage, but we have to be compatible with it.

@tleunen
Copy link
Owner

tleunen commented Sep 21, 2020

I've never seen really from '.', but I've seen from './' in many projects, so if that's an issue, I definitely agree we need to fix it :) Thank you @relign!

@fatfisz You good with merging this?

@juzerzarif
Copy link

@tleunen @fatfisz Any ETA on when this will be merged/released?

@fsmaia
Copy link

fsmaia commented Feb 18, 2021

@fatfisz @tleunen up

@fsmaia
Copy link

fsmaia commented Feb 18, 2021

Just for curiosity: why not !path.isAbsolutePath()?

@relign
Copy link
Author

relign commented Feb 25, 2021

Just for curiosity: why not !path.isAbsolutePath()?

path.isAbsolute('.') is false.
so far, import xxx from '.' is the only problem.

@nwalters512
Copy link

@tleunen @fatfisz can I do anything to help push this along? Just started using this excellent package, but this bug is causing pretty serious issues for us.

@htang014
Copy link

Hi all, has this plugin been abandoned? Have not seen any activity in months and this PR has been open since then

@Grohden
Copy link

Grohden commented Feb 17, 2023

@tleunen
Copy link
Owner

tleunen commented Feb 17, 2023

@Grohden It's allowed so it should definitely be supported. I agree. Even if it's kind of risky of having circular dependencies.

@Grohden
Copy link

Grohden commented Feb 17, 2023

Yo, suggestion for maintainers: use prettier or dprint to format the project so we don't have unnecessary changes/noise at PRs (like formatting)

Also, for people looking for a patch (for patch package) here's the simple version (its just the if, its basically the same for 4.1.0)

babel-plugin-module-resolver+5.0.0.patch

diff --git a/node_modules/babel-plugin-module-resolver/lib/resolvePath.js b/node_modules/babel-plugin-module-resolver/lib/resolvePath.js
index cd53e95..5f17543 100644
--- a/node_modules/babel-plugin-module-resolver/lib/resolvePath.js
+++ b/node_modules/babel-plugin-module-resolver/lib/resolvePath.js
@@ -75,7 +75,11 @@ function resolvePathFromAliasConfig(sourcePath, currentFile, opts) {
 }
 const resolvers = [resolvePathFromAliasConfig, resolvePathFromRootConfig];
 function resolvePath(sourcePath, currentFile, opts) {
-  if ((0, _utils.isRelativePath)(sourcePath)) {
+  // https://github.com/tleunen/babel-plugin-module-resolver/pull/409
+  // `.` and `./` are specific paths, we can't transform them
+  const isSpecificPath = sourcePath === '.' || sourcePath === './'
+
+  if ((0, _utils.isRelativePath)(sourcePath) || isSpecificPath) {
     return sourcePath;
   }
   const normalizedOpts = (0, _normalizeOptions.default)(currentFile, opts);

btw, I wonder what kind of changes can be done to make it easier to debug whats being transformed.. I spend the whole day trying to figure out why my test was ending up with a circular import (undefined import values) in a unrelated file.. I at least added a debug flag for my own plugin

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.

None yet

8 participants