-
-
Notifications
You must be signed in to change notification settings - Fork 5.6k
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
Extend babel-template to work as a template tag #5001
Conversation
} else { | ||
return template(firstArg, ...rest); | ||
} | ||
} |
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 tried making a separate export instead, but that would be a breaking change because of add-module-exports
.
Can we get rid of it? 🙂
Current coverage is 89.14% (diff: 98.14%)
|
packages/babel-template/src/index.js
Outdated
} | ||
|
||
function template (partials: Object | string[], ...args: Array<Object>) { | ||
if (!Array.isArray(partials)) { |
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.
Should we throw
if args.length > 0
for this case to help in case people mistype something?
} | ||
|
||
if (typeof arg === "number") { | ||
replacementMap.set(arg, `$${arg}`); |
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.
Do we want $BABEL_TEMPLATE$
on this too? Or is this already an assumption with the current template implementation?
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.
This is using the current implementation's positional argument replacement.
It is validated that 0
cannot be used if named arguments are used too, but I forgot to check for /$\d+/
in the string parts themselves...
I think this would be a great addition for 7.x, @Kovensky Would you have time to fix the conflict? |
Supports: ```js // these all should produce "code;" when generated template`code;`(); template`${0}`(t.identifier('code')); template`${'code'}`({ code: t.identifier('code') }); template`${t.identifier('code')}`() template({})`code`(); ```
7d6a6fc
to
19dfe64
Compare
Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/5183/ |
This is a great addition IMO, can it be documented in the README? |
Yea we should make an issue about it; it can go in the 7.x post too #6388 |
Supports: