-
Notifications
You must be signed in to change notification settings - Fork 93
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
Support passing an alias object to fix issues with babel module resolver #17
base: master
Are you sure you want to change the base?
Conversation
This one need to be merged @kesne |
README.md
Outdated
{ | ||
"alias": { | ||
"plugins": [ | ||
"root": "./", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the syntax here seems a bit odd. In the PR description you have something like:
["inline-react-svg", {
"root": "./",
"alias": {
"svgs": "svgs"
}
}],
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are completely right, the plugins array is repeated, it shouldn't be there
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ooHmartY Fixed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome! Thank you 👍
@kesne Would it be possible to get this merged. Thanks! |
@Zaggen This looks great, any chance you can rebase on master? |
package.json
Outdated
@@ -1,6 +1,6 @@ | |||
{ | |||
"name": "babel-plugin-inline-react-svg", | |||
"version": "0.4.0", | |||
"version": "0.5.0", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll make this change when I cut a new release, no need to make it here.
README.md
Outdated
@@ -65,9 +65,28 @@ npm install --save-dev babel-plugin-inline-react-svg | |||
] | |||
] | |||
} | |||
``` | |||
- *`alias`* - An object with alias for module resolution | |||
- *`root`* - A relative path string (Starting from CWD), it only works in conjunction with alias. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm confused why you need this, can't you just define the full paths you care about in the alias?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You mean the root? I can change it to work with out, so its optional.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm mostly confused why you need this at all, can't the aliases include the fully qualified path?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@kesne If I may to add my 5 cents: babel-plugin-module-resolver uses root
to specify the common prefix for all aliases, which is convenient when config and paths you alias are located in different places (otherwise, root should default to CWD). Moreover, root
there accepts an array of paths, or glob path: this way it allows to look for aliases in multiple directories.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems like we should allow root
to be defined always then, not just in conjuction with alias.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But without alias
it is not supposed to do anything, I guess.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@kesne @birdofpreyru Then what should be the best option here? . Indeed babel-plugin-module-resolver
uses root, I believe I added for compatibility reasons, thought that module uses an array as mentioned before. As @birdofpreyru mentioned, the root property is useless without any aliases. If I'm not wrong the other plugin requires that you always specify the root, right now it works like this too, so I'm not sure now if we should change this or not, let me know what you think.
@kesne Did a rebase, but I din't get a chance to properly check if it doesn't break the functionality, I'll make the changes with the root property as soon as I have time and then you can confirm if everything is ok |
Co-authored-by: Zaggen <[email protected]> Co-authored-by: Oscar Barrett <[email protected]>
Anything I can do to help move this PR along? Seems like there hasn't been any activity in a while but the functionality seems super useful. |
I've got a working branch based on this PR, just haven't gotten around to submitting it. With an example config being ['babel-plugin-inline-react-svg', {
root: path.resolve(__dirname, 'src'),
alias: {
images: 'images'
}
}], IIRC |
@OscarBarrett Could you delivery package since airbnb is sleeping You can bypass the problem in typescript by reexporting import AppStore from '../static/appstore.svg' then just |
@patsadow2 nobody's "sleeping". |
I've pulled in @OscarBarrett's rebased branch. This will still need tests. |
Hi guys, as of now, a nextjs project with svg, absolute path and Typescript cant import a svg like Is this still happening? |
when it will be merged? |
@vavilondev when someone posts a link to a branch that contains tests. |
Given a .babelrc like this
Supports basically the same syntax as module-resolver but root has to be a string not an array
Closes #14
Let me know what do you think