Skip to content
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 new babel feature allowing plugins to indicate external dependencies #49

Open
TSMMark opened this issue Feb 4, 2022 · 7 comments

Comments

@TSMMark
Copy link

TSMMark commented Feb 4, 2022

Hey Kent, thanks for this awesome plugin, been using it for years and it's really handy! Also saw you got in a car accident recently and I hope you're not too beat up and recovering well.

I've been following this feature which just got merged to babel: babel/babel#14065

I think that might be useful for this plugin to implement. It could allow us to invalidate the codegen output if the code has a dependency on some files on disk.

It seems pretty simple to utilize the new feature, if I'm right you just call api.addExternalDependency(external) where external is a filepath. https://github.com/babel/babel/pull/14065/files#diff-02893c5f57e6e79d081162d32cbfe84ccfac5ed70b8933be563e1dd7256f179cR15


Here's an example of a codegen module whose result depends on files on the filesystem, and could benefit from external dependency awareness:

// @codegen
const join = require('lodash/join')
const replace = require('lodash/replace')
const glob = require('glob')
const path = require('path')

const examplesFiles = glob.sync(path.resolve(__dirname, '**/examples.*'))

export default `export default {
  ${join(examplesFiles.map((exampleFile) => {
    const chopSrcFromPath = replace(exampleFile, /(.*):?src\//, '')
    return `'${chopSrcFromPath}': require('./${chopSrcFromPath}')`
  }), ',\n')}
}
/* 02-03-2022-09:22:00 */`

Right now we resort to updating the timestamp comment to force an invalidation + codegen rebuild. (Maybe there's a better way that I don't know about?)

I'm not sure the internals of this library but do you think it makes sense to implement? I might be able to take a stab at implementing it. Let me know if you have any pointers. Thanks!

@kentcdodds
Copy link
Owner

Cool! I'd be willing to merge a pull request for this. I don't have time to work on it personally though.

@TSMMark
Copy link
Author

TSMMark commented Feb 7, 2022

Great, so I've taken a look at the internals of this package to figure out how this would be implemented, and I can imagine a few potential approaches, but here's what I'm thinking might be best:

in getReplacement helper it seems like codegen module may export a function and receive args:

module = module(...args)

This seems like an ideal place to inject the babel core function addExternalDependency as a function argument to be invoked dynamically inside the codegen script.

So the example from #49 OP might look more like this:

// @codegen
export default ({ addExternalDependency }) => {
  const join = require('lodash/join')
  const replace = require('lodash/replace')
  const glob = require('glob')
  const path = require('path')
  
  const examplesFiles = glob.sync(path.resolve(__dirname, '**/examples.*'))
  examplesFiles.forEach(addExternalDependency)

  return `export default {
    ${join(examplesFiles.map((exampleFile) => {
      const chopSrcFromPath = replace(exampleFile, /(.*):?src\//, '')
      return `'${chopSrcFromPath}': require('./${chopSrcFromPath}')`
    }), ',\n')}
  }`
}

Does that make sense to you or do you have any better ideas? I'm not entirely sure how args is supposed to be used currently so if this doesn't play nice with existing functionality that would be an issue.

@kentcdodds
Copy link
Owner

I suppose that makes sense for the comment form. We may need a different approach for the other forms though. I wonder if we could just reference it as a global variable instead?

@TSMMark
Copy link
Author

TSMMark commented Feb 8, 2022

Okay I'll have to do more research on how babel-plugin-codegen works in its other forms then, because I've only used it in the comment form.

A global variable would probably work and is something I had considered but it did seem a little smelly to me. I'm also not sure if it's possible to set it up so that every separate dynamic/codegen file would have its own local instance of the global variable, because addExternalDependency could be invoked by multiple dynamic/codegen files, and each file should have its own differentiated set of dependencies.

So I imagine there's got to be a way to declare a global variable so that it will only be available within the execution context of building/codegen-ing one file... so each file would have its own set of 'external dependencies'.

Do you think you could share a simple example of how you're imagining a global variable could be used for some of the forms other than the comment form?

Another option may be to not support addExternalDependency in every form of babel-plugin-codegen initially — not sure how you feel about that tho.

@goatandsheep
Copy link

heads up: make sure you use 7.17.2 as there was a bug in 7.17.0

@kentcdodds
Copy link
Owner

Everything about compiler plugins is "smelly" 😅 I honestly think it's the cleanest solution. I wonder whether it would be bad to call addExternalDependency with the same file multiple times. If it is bad in practice you could have your own global that keeps track of the calls and only calls the actual function with unique values.

As an example (from the README):

import /* codegen(3) */ './assign-identity'

The assign-identity module could simply call addExternalDependency for any deps it has.

Heck, maybe even overloading the global require would be interesting so folks wouldn't have to call it manually 🤔

But this is as much thinking as I have to dedicate to something that I'm never going to use. Good luck!

@TSMMark
Copy link
Author

TSMMark commented Mar 28, 2022

I tried implementing this but it seems like external dependencies must be added before the plugin is returned and registered to babel (e.g. right here:

const {asProgram, asIdentifier, asImportDeclaration} = getReplacers(babel)
), and the cache.using entries can't be added / updated dynamically after the plugin has been returned.

So that leads me to the assumption that this feature proposal will not be possible with the current version of the caching feature of babel... However, I'm hoping I'm wrong and that someone more familiar with babel core will prove me wrong eventually, or the babel caching will be updated to support dynamic cache deps.


FTR The specific error is closely related to this open issue babel/babel#14252

It seems like the babel cache/deps are intented only to be used for a static set of files, all of which are known at plugin definition time, and can not be dynamic, and can not be delayed until compile time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants