redwoodjs / redwood

The App Framework for Startups
https://redwoodjs.com
MIT License
17.33k stars 994 forks source link

[RFC] Setup external modules and scripts with `yarn rw setup plugin` #2086

Open dac09 opened 3 years ago

dac09 commented 3 years ago

This RFC proposes:

Allow setup commands to run external node_modules and external scripts gists.

In order to achieve this we would need the following

1. Setup a standard that all redwood setup packages must conform to

Update: my thinking around this has evolved a little, see comment here https://github.com/redwoodjs/redwood/issues/2086#issuecomment-950957952

const RedwoodSetupPlugin = {
    minimumVersion: '0.27.0' // setup will check against this version
    preinstallChecks: () => {
    // add custom logic to check if project is suitable for running this setup plugin against
    // e.g. plugin author may want to check presence of certain configuration files
  }, 
    steps: [
        {
        description: 'Step that describes what im doing', 
        command: 'yarn install bazinga' // string to execute in cli or function
      }, 
        {
        description: 'Running more involved steps here', 
        command: () => {
                console.log('Im going to have some awesome custom logic here')
                prompt('Lets confirm you want to do yyy')
                fs.copySync(/* xxx */)
      }
      },    
    ] 
}
export default RedwoodSetupPlugin

2. Add new command yarn rw setup plugin xxx

Where xxx can be: a) A node module e.g. yarn rw setup plugin redwood-setup-foo b) A gist e.g. yarn rw setup plugin https://gist.github.com/foo/bar c) A local node module e.g. yarn rw setup plugin ~/Code/redwood-setup-bar d) A github repo, treating the code as a node_module yarn rw setup plugin github.com/xxx/redwood-setup-foobar

3. Handle setup plugin modules

Step 0: Determine if this is a gist, local setup or npm module Install module. Note that this will modify the package.json and lock file. Is this something we can avoid?

If its a node_module, perhaps the workflow can be a) Install redwood-setup-plugin-bazinga b) Run this new setup plugin with yarn rw setup plugin bazinga

Step 1: Check if current project redwood is at atleast minimum version of redwood

if (matchesMinimumversion(mySetupModule.minimumVersion, project.redwoodVersion)) {
    // continue without warning
} else {
    console.log('The last version this plugin was checked against was ${mySetupModule.minimumVersion}')
    prompt('Continue with setup?')
}

Step 2: Run preinstall checks

mySetupModule.preinstallChecks()

Step 3: Run setup steps

mySetupModule.steps.forEach(setupStep => {
    console.log(setupStep.description)

    if (typeof setupStep.command === 'function') {
        setupStep({
            paths, // pass paths object from @redwoodjs/internal
            projectConfiguration // pass redwood.toml
        })
  } else {
        // if its a string, simply pass it to 
        execa(setupStep.command}
  }
})

We could potentially use listr to execute the steps, but we would have to be check that listr still allows interactivity, incase they have a prompt as one of the steps.

thedavidprice commented 3 years ago

Thanks! Prioritizing this for next sprint

jtoar commented 3 years ago

Linking to the auth discussion here: https://github.com/redwoodjs/redwood/issues/2260#issuecomment-817947281.

realStandal commented 3 years ago

I know this post is going to be wordy - apologies - but I really love the implications this feature has for the framework.

Please feel free to ignore this post until the bandwidth is available :)

I love the implications so much so that I even went as far as to mock an implementation.

Much of the remainder of this document will be about explaining this implementation - some information is replicated from the README linked above. At the very end are some of my suggestions which add to Danny's original post.

setup.js Standard

I'll start the same way Danny did, my proposal for the standard setup plugins should adhear to.

Compared to what was originally suggested, I've made a few minor changes:

Fields with a ? suffix are optional. Defaults are undefined, unless otherwise stated.

const RedwoodSetupPlugin = {
  redwoodVersion: '0.37.4',
  preSetup?: async () => {},
  /**
   * `steps` itself is optional, but if the field is present, at least one step MUST be defined.
   */
  steps?: [
    {
      /**
       * The `Listr` title to display for this step.
       */
      title: 'Running a simple command.',
      /**
       * A `function` or `string` defining this step's logic.
       *
       * * When a `function`, the result of calling that function will be passed to the underlying `Listr`.
       *   * The function's first argument will be the [context](https://www.npmjs.com/package/listr#context) of the underyling Listr.
       *   * The function's second argument will be the [task object](https://www.npmjs.com/package/listr#task-object) of the underlying Listr.
       * 
       * * When a `string`, the value is executed as a command.
       */
      task: 'yarn ...',
      /**
       * How to handle an error if one should occur when running this step.
       *
       * * `break` - End execution and skip any remaining steps, jumping straight to `postSetup`.
       * * `continue` - Continue to the next step, as though this one completed successfully.
       * * `fail` - Immediately stop execution of the setup script.
       *
       * @default - 'fail'
       */
      errorHandling?: 'continue',
      /**
       * A custom message to display should *any* error occur running this step.
       */
      errorMessage?: 'An error occured.',
      /**
       * A function to be ran **before** a step's error has been handled using the method listed under `errorHandling`.
       */
      onError?: async (err: Error) => {},
    },
  ],
  postSetup?: async () => {},
}

module.exports = RedwoodSetupPlugin

Plugin Loading

This section is in a "well it works" state - personally, I have no intentions of this remaining as-is. I'd appreciate guidance on a better way to deduce source if I were to personally see the issue through :)

As the list shows, plugins are always downloaded (or copied) to the application and then their setup is executed. By default, if a plugin already exist it is not re-downloaded - making it possible to download a plugin, make modifications, then run the setup script but preferring your local changes.

Plugins can be downloaded only, skipping execution of their setup-script, by using the --skip-setup (-s) option. If a plugin has already been downloaded, the --force (-f) option can be used to force redownloading the plugin, overwriting any local changes.

Version Assertion

As the original post stated, a check is made comparing the Redwood version specified in a plugin's setup-script to the current application's. I've added the compare-versions dependency to expedite this process - it has quite broad support for the Semver specification.

To make recursive invocations of rw setup plugin possible, I've also added the --auto-yes (-y) option. This will automatically enter "yes/confirm" if a version inconsistency is encountered.

Demos

Everyone loves demos, right?

Author's Notes

Just some ideas I jotted down/thought of while doing all this:

dac09 commented 3 years ago

Hey @realStandal - thanks for taking this idea for a spin! I've actually been thinking about this a little differently lately. My current thinking around this:

  1. Enable a plugin by running yarn redwood plugin add redwood-plugin-bazinga This will install a nodemodule for example (TBD), but importantly add this to the redwood.toml.
[api]
  port = 8911
  [api.paths]
  functions = './api/src/functions'
  graphql = './api/src/graphql'
  generated = './api/generated'
+[plugins]
+ - "redwood-plugin-bazinga" 

Why? Because we don't want to blindly be searching for plugins, and also makes the whole piece a little more secure (i.e. plugins are opt-in, rather than autodetected)

  1. The cli will then use yarg's commandDir, to load up additional cli functionality from the plugin.

We do this internally too e.g. in generate https://github.com/redwoodjs/redwood/blob/main/packages/cli/src/commands/generate.js#L15

And read additional CLI options from the listed plugins. In essense the logic being like:

for each plugin in redwood.toml 
  loadYargsHandlersForPlugin(plugin)

Why? This gives a lot more flexibility in the plugin side, i.e. you can have one command, or multiple commands, AND you get it in the CLI help screen too.

For example:

yarn rw plugin bazinga doSomething

We could potentially also skip the plugin namespace


What do you think?

realStandal commented 3 years ago

Hey @dac09, thanks for taking the time to get back to me so quickly.


I assume with this revision we'd move out of the realm of strictly setup plugins, and into more general-purpose land? I think enforcing some level of version-control would be necessary at this point.

If setting something up is part of my plugin, I have yarn rw plugin my-plugin setup.

If so, I think a "Redwood's Conventions" would be helpful: If you're writing a command that does this, name it this to fit with the rest of the framework and (hopefully) other plugins.


...importantly add this to the redwood.toml

Would this definition act in the same way as dependencies in package.json? That is to say: If you and I are working on an application and I add a new plugin - will you be able to yarn install and have that plugin available? Or can running the plugin just take my download-if-no-local-copy-exist flow?

The cli will then use yarg's commandDir, to load up additional cli functionality from the plugin.

I had the same thought while implementing what was originally posted - thank you for putting it eloquently and showing me I can go outside the lines if I think there's a better choice :)

We could potentially also skip the plugin namespace

I'm on the fence. I appreciate the explicit-ness of what it is you're running when invoking the command. But, my fingers appreciate the 6 less keys.

dac09 commented 3 years ago

If so, I think a "Redwood's Conventions" would be helpful: If you're writing a command that does this, name it this to fit with the rest of the framework and (hopefully) other plugins.

I agree with this totally - but I see this being more guidance than enforcement. i.e. not all plugins will need to have a setup, install or run. But our plugin writing guide could say for example

"if you're changing any code, or adding new files to a user's project call this setup "

Would this definition act in the same way as dependencies in package.json?

I was imaginging that it would actually install a module (and update the package.json in the process). So the version control would still come from package.json, but the toml is just an extra opt-in step, to make sure the redwood cli doesn't just execute anything with a "redwood-plugin-*" name (which was my original suggestion)

If someone else in your team runs the redwood cli, they would have the same setup as you, because you'd have pushed up the three musketeers of plugins: package.json, yarn.lock and redwood.toml

The outstanding question here would be how we could support gists (github repos is easy, thats the same as a npm module)

I'm on the fence. I appreciate the explicit-ness of what it is you're running .. But..

yeah me too!

realStandal commented 3 years ago

I see this being more guidance than enforcement

100% agree.

If someone else in your team runs the redwood cli, they would have the same setup as you, because you'd have pushed up the three musketeers of plugins: package.json, yarn.lock and redwood.toml

👍

The outstanding question here would be how we could support gists

My thoughts too, the ease of just throwing a Gist up is something at least worth looking into.

Referencing this article, Gists can be installed using NPM, similar to how GitHub repositories are. Taking it a step further, this Stackoverflow comment highlights how the gist can be aliased in (at least) two ways:

The issue I see is that it doesn't look very straightforward how we might be able to ensure the version of a Gist installed is the one that will always be used. The yarn add ... command references the Gist's underlying git-repository, but I'm not finding a way to target a specific revision's - not sure if you even can from that interface.

I wrote this response before what I wrote what I did above, keeping for the sake of keeping:

Just looking at the API, it does look like GitHub makes it pretty straightforward to target a specific revision of a Gist. When you hit https://api.github.com/gists/{gist id} it will provide you with a list of that Gist's history. Each member of that array has a unique version ID, which can be used to pull up the Gist at that point in time.

There is also a https://api.github.com/gists/{gist id}/commits endpoint, which will just list the Gist's up-to-date revision-history.


On the namespace subject, is aliasing plugin as simply p too generic? It'd be pretty easy to introduce the full-form the first time it's referenced in the docs, then add a "You may also use the short-form, p, when invoking a plugin" then stick with it.