-
Notifications
You must be signed in to change notification settings - Fork 135
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
Prevent macro from running without babel-marcos #20
Comments
Hmmm... What if you do: // my.macro.js
const macroLogic = require('./macro-logic')
module.exports = macroLogic({isBabelMacros: true})
// server-thing.js
const macroLogic = require('./macro-logic')
module.exports = macroLogic({isBabelMacros: false}) I never really imagined macro running as anything except a build-time thing. I'm not certain that I understand the use-case... Could you help me understand? |
Hahaha, it's not really a "use-case" I guess, rather a helpful error But I found myself requiring the macros without having babel-macros installed, and think it would be nice to have an helpful error there :) |
Gotcha. How do you suppose we could avoid requiring a macro from doing this kind of check manually? My guess is that most wont do it. Is there any way we could just make it work without requiring macro authors from doing it manually? |
Yeah, been thinking about that too, but couldn't figure out a way. Maybe most of them won't do it, but I certainly will. With all the build setup problems, it is too easy to include the macro in a place where it is not being macro-ified, and the error you get by default isn't very helpful either, hahaha. Then again, I could also check for the state or ref object if you don't want to add this :) |
Nah, let's add it. Could you do that? I'd probably just change this to this: macros({
references: referencePathsByImportName,
state,
babel,
isBabelMacrosCall: true,
}) Then add docs about this property encouraging macro authors to add a check at the start of their macro function and throw a helpful error if that property does not exist. |
Just talked with @threepointone and he suggested that we change the API for defining a macro slightly: const createMacro = require('babel-macros/create') // or something like that
module.exports = createMacro(({references, state, babel}) => {
// do macro stuff
}) And the module.exports = createMacro
function createMacro(macro) {
return macroWrapper
function macroWrapper(options) => {
if (!options.isBabelMacrosCall) {
// throw error
}
return macro(options)
}
} This way folks don't have to worry about whether they're running in a compilation process or not. I think this is a great solution and interestingly it's not a breaking change FWIW... If anyone wants to take this please be my guest :) I'll ping in here when I'm ready to start working on it if nobody else has. |
I'd be happy to take this on! |
Awesome! Please make sure to update docs! Also, I realized that the authoring docs doesn't have an example. Could you put an example in there? You could base it on one of the examples from the unit tests 👍 |
I'm going to start working on this |
Hey, sorry for the delay on this. I got hung up with some personal stuff. Sorry for not being able to complete it. |
No worries :) We're all just doing what we have time for 😄 |
Closes #20 This also upgrades all tooling to kcd-scripts
This also upgrades all tooling to kcd-scripts Closes #20
This also upgrades all tooling to kcd-scripts Closes #20
Now it's recommended to wrap your macro in |
Not "recommended" ... "required" 🕶 |
I was wondering, as I am using babel-macros in a project now, if there could be a way added so macros could "catch" it when they are being imported in a normal file.
As it seems now, there isn't such thing. The
this
is unset, if packing with webpack I could check if I am running in node (browser won't), but I also use this on node. Possible would be to check the arguments, but as the mantra of this repo is, I think explicit is better than implicit ;)I wonder what the best way is though:
this
seemed good, but won't work with arrow functionsSo who cares about hacky 😅 (This one is actually also possible (cleaner?) with Symbols)
Most likely I am missing an option or some perspective, so I'd like to discuss this :)
The text was updated successfully, but these errors were encountered: