-
Notifications
You must be signed in to change notification settings - Fork 17
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
✨ add support for babel-plugin-macros #64
base: master
Are you sure you want to change the base?
Conversation
Hi @leonardfactory can you look at this ? |
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.
Hi and thank you for your contribution! I left my two cents here
package.json
Outdated
"peerDependencies": { | ||
"babel-plugin-macros": "^3.1.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'm a bit worried about this, since babel-plugin-macro
is not strictly a dependency, but an optional one. An idea would be to use peerDependenciesMeta in order to specify they're optional, even if this would require additional checking in actual code. Can you take a look at this?
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.
Sounds reasonable.
@leonardfactory I made the babel-plugin-macros dependency optional |
Hi @intellild, I didn't have time to analyze this correctly before; now I'm wondering which use case you are trying to cover here. I'm genuinely curious about your proposal, I'll try to explain my point of view. The plugin works well when you need to work with
Furtermore this plugin tries to emulate (with a best-effort strategy) the official TypeScript feature Given all of that, I'm curious about the macro usage and I'd like to hear back from you before including it in the package – I'm just worried it could bring a bit of confusion to future and existing users. |
@leonardfactory
|
support babel-plugin-macros
example: