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

Escape key for alias to support using $ prefix in place of @ #312

Closed
Telokis opened this issue Aug 18, 2018 · 6 comments
Closed

Escape key for alias to support using $ prefix in place of @ #312

Telokis opened this issue Aug 18, 2018 · 6 comments

Comments

@Telokis
Copy link
Contributor

Telokis commented Aug 18, 2018

In my config I use the $ prefix to point to my own directories and avoid having to type ../../../ everywhere.
Here is an example:

"alias": {
    "$src": "./src",
    "$common": "../Common",
    "$config": "./config",
    "^\\$(.+)": "./src/\\1"
}

The last one will simply convert anything beginning with $ to a directory/file inside my src directory. ($helpers/ becomes ./src/helpers/)

I noticed that you don't regex escape the key before using it.

Would you accept a PR to fix this?

By the way, for anyone looking for a solution to this problem, you can manually escape your keys in the config. In my case, it will become:

"alias": {
    "\\$src": "./src",
    "\\$common": "../Common",
    "\\$config": "./config",
    "^\\$(.+)": "./src/\\1"
}

and this works.

Telokis added a commit to Telokis/babel-plugin-module-resolver that referenced this issue Aug 18, 2018
@tleunen
Copy link
Owner

tleunen commented Aug 22, 2018

@fatfisz - You pretty much coded this feature with regex. Would you mind taking a look? :) Not sure if there were any reason to that?

Not sure what to do when you really want to use one of these specific regex characters. Like.. Forcing the end of the regex with $? But again, how useful is it?
With babel 7 and a js file as a config, we could support regexes out of the box instead as well.

@Telokis
Copy link
Contributor Author

Telokis commented Aug 23, 2018

To me it really seemed to be a simple forgotten detail in the code. The current handling is nice and I submitted #313 to fix the issue and add a test case.
You can't force users to use a js config if they want to use regexes. I think the current way of handling is not bad since nobody complained about it!
In my case, the $ is at the beginning of the match so it doesn't trigger a regex matching.

@tleunen
Copy link
Owner

tleunen commented Aug 23, 2018

Oh, I believe you're right @Telokis. Thanks for the additional info. I'll take a deeper look in the next few days.

It doesn't have anything to do with the regexes actually. Not the ones we detect when it starts with ˆ or ends with $. So that's why I was all mixed up. But I believe you're right with the escaping here.

@fatfisz
Copy link
Contributor

fatfisz commented Aug 23, 2018

Hi, I'll be able to finally take a look at this today, later in the evening.

@fatfisz
Copy link
Contributor

fatfisz commented Aug 23, 2018

Yup, it looks like I overlooked this. Thanks for contributing the fix ❤️

@Telokis
Copy link
Contributor Author

Telokis commented Aug 23, 2018

My pleasure!

fatfisz pushed a commit that referenced this issue Aug 23, 2018
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

3 participants