kentcdodds/babel-plugin-macros

Prevent macro from running without babel-marcos

dralletje opened this issue ยท 13 comments

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:

  • Setting this seemed good, but won't work with arrow functions
  • Setting a property to check on the macro function itself:
      const macros = require(requirePath)
    + macros.with_babel_macros
      macros({
        references: referencePathsByImportName,
        state,
        babel,
      })
    
    Feels kinda hacky, but again we are putting macros into javascript,
    So who cares about hacky ๐Ÿ˜… (This one is actually also possible (cleaner?) with Symbols)
  • Passing a symbol as first argument that can be compared
    import babelMacros from 'babel-macros';
    export default ({ is_babel_macros, references, state }) => {
      if (is_babel_macros !== babelMacros.is_babel_macros) {
        ...
    

Most likely I am missing an option or some perspective, so I'd like to discuss this :)

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 createMacro function would be something as simple as this:

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 ๐Ÿ˜„

Now it's recommended to wrap your macro in createMacro. See the updated docs ๐Ÿ‘

Now it's recommended to wrap your macro in createMacro. See the updated docs ๐Ÿ‘

Not "recommended" ... "required" ๐Ÿ•ถ