Giter Club home page Giter Club logo

Comments (16)

DavidWells avatar DavidWells commented on May 18, 2024 1

Thanks for the write up. I'm all for simplifying.

I think having 2 different words to load a plugin path and name might be confusing.

In any case, I've been thinking about the plugin shape for a while and plugins might need to be an object rather than an array.

plugins:
  pluginXyz:
     name: foo-bar
     optionOne: 'hi
  pluginZaz:
     name: otherThing
     optionOne: 'hi

There are a couple reasons for this:

  • Support for inputs & outputs to plugins
  • Building a DAG of the lifecycle and making sure things run in the correct order
  • Allowing for plugins to be used multiple times with different inputs (and output value refs)

For example. Lets say a mongo-db plugin returns a connection string as an output. Then a second plugin mongo-db-crud-function relies on that connection_string value returned from the first plugin.

We need a way for values to be referenced from plugins in other plugins.

Example:

plugins:
  pluginXyz:
     name: mongo-db
     optionOne: hi
  pluginZaz:
     name: mongo-db-crud-function
     mongoURL: ${pluginXyz.connection_string}

It's not really possible to reference values easily from a position in an array

from build.

DavidWells avatar DavidWells commented on May 18, 2024 1

Sounds good! I will submit a PR

Hold off on PRs on this for right now 😃. Need to figure out the shape


The object notation I described above works like this:

plugins:
  myLogicalPluginName:
    name: plugin-xyz
    foo: wowow
  myOtherLogicalName:
    name: plugin-xyz
    foo: coooooollllolololo

Here the plugin plugin-xyz is loaded twice with different params. The logical ids (aka parent keys) are different and therefore can be referenced individually like ${myLogicalPluginName.outputXyz} and ${myOtherLogicalName.outputXyz}

The name (or it could be type) field, is referencing the module name or the path to the import.

Local plugins are an important part of the story and I'm not to crazy about

plugins:
  myLogicalPluginName:
    name: ./my-local/plugin-xyz
    foo: wowow

On the other side, I think it is also confusing to have multiple keys name, import (orresolve)

Some design goals are:

  • Keep it super simple to grok at first glance
  • Flat as we can. You're note on extra indentation is good one here
  • Flexible enough to support advanced use cases. (using plugin multiple times, outputs, resolving a D.A.G., etc)

There are some advanced configuration techniques possible but I'm not certain we want to expose those just yet.

from build.

swyxio avatar swyxio commented on May 18, 2024

agree in principle.

instead of name: and path: we might call it resolve:

from build.

ehmicky avatar ehmicky commented on May 18, 2024

Yes that makes sense.

We could theoretically allow indexes in the notation (${plugins[0].pluginXyz} or ${plugins.0.pluginXyz}) but I agree that's not as user-friendly as referencing plugins by name.

Sounds good! I will submit a PR to go from:

plugins:
  - netlify-plugin-functions:
      enabled: false
  - ../../my-plugin:
      enabled: true

To:

plugins:
  netlify-plugin-functions:
    enabled: false
  ../../my-plugin:
    enabled: true

from build.

ehmicky avatar ehmicky commented on May 18, 2024

I just thought of the two following issues:

  • users might want to call the same plugin several times but at different times or with different options. That's not possible with an object since duplicate keys would be ignored.
  • using a dot notation with ${...} does not work when the plugin is loaded by file path, because the file path contains dots.

One possible solution to both issues would be to introduce an optional import property to plugin configuration objects:

  • its value is either the module name or the file path to the plugin
  • it defaults to the plugin configuration object's key

Example:

plugins:
  # 'import' is optional and defaults to the configuration object key
  my-plugin:
    enabled: true
  ../../plugin/path:
    enabled: true
  # 'my-plugin' is used again but with different options
  samePluginAgain:
    import: my-plugin
    anyOption: true
  # This works with ${...}
  localPlugin:
    import: ./path/to/local/plugin
  otherPlugin:
    import: ${plugins.localPlugin.import}

What do you think?

from build.

DavidWells avatar DavidWells commented on May 18, 2024

Also something to keep in mind: the syntax/design of this will likely extend for the addons section of config as well.

addons:
  FaunaOne:
    name: fauna
    dbName: todos
  FaunaTwo:
    name: fauna
    dbName: users

from build.

ehmicky avatar ehmicky commented on May 18, 2024

@dwells I like your idea!

So if we summarize, we would go from:

plugins:
  - moduleName:
      option: true
  - ./local/path:
      option: true

to:

plugins:
  pluginId:
    name: moduleName
    option: true
  otherPluginId:
    name: ./local/path
    otherOption: true

Did I get this right?

Three small follow up questions:

  1. Isn't the word name is a little odd here where it is refering to a file path?
  2. Can users omit the name if it's the same as the "logical id" (parent key)?
  3. At the moment we prepend @ to scoped module names. I am assuming that's because YAML does not allow keys to start with @ (unless escaped). However with this new syntax, that's not a problem anymore, so should this @-prepending behavior be removed? I think it would be clearer for the users to prepend the @ themselves.

from build.

DavidWells avatar DavidWells commented on May 18, 2024
  1. Isn't the word name is a little odd here where it is refering to a file path?

Indeed. Perhaps type? Open to ideas 😃

  1. Can users omit the name if it's the same as the "logical id" (parent key)?

Maybe. It might get weird referencing logical ids with dashes. ${my-plugin-xyz.outputXYZ}. But I think this could work.

name (or maybe type) could potentially have version number as well. Example name: [email protected]. This would be to support the (unlikely) usecase of using the same plugin of different major versions. Something to think about 🤔

the moment we prepend @

Yeah yaml yells at you for those and you have to wrap in quotes. Which is ugly (IMO)

Yeah we can remove that "magic" fixer and have people specify the full path @netlify/whatever-plugin as the value. That would actually make things clearer on what its referencing

from build.

ehmicky avatar ehmicky commented on May 18, 2024
  1. Indeed. Perhaps type? Open to ideas

Since the value is directly passed to require(), what about require, import or resolve? Or from, module, main or entry?

  1. Maybe. It might get weird referencing logical ids with dashes. ${my-plugin-xyz.outputXYZ}. But I think this could work.

This would allow users to write only:

plugins:
  my-plugin:
    option: true

instead of:

plugins:
  my-plugin:
    name: my-plugin
    option: true

Still not sure whether being concise (first example) or being explicit (second example) is better developer experience there... Just let me know what you think, and let's go with that :)

  1. name (or maybe type) could potentially have version number as well. Example name: [email protected]. This would be to support the (unlikely) usecase of using the same plugin of different major versions. Something to think about thinking

👍

Yeah we can remove that "magic" fixer and have people specify the full path @netlify/whatever-plugin as the value. That would actually make things clearer on what its referencing

👍

  1. Finally I though of the following: let's say we try to enforce the netlify-plugin-* package name convention.
  • Could we allow users to omit the netlify-plugin- prefix from the plugin name in the configuration file? ESLint for example does that.
  • If we do this, should omitting that prefix be optional or mandatory? Making it mandatory creates less ambiguity for the user and also enforces the package name convention.
  • If we do this, how would this apply to scoped packages?

from build.

DavidWells avatar DavidWells commented on May 18, 2024

I'm leaning more towards more explicit the better.

Playing around with a format.

Notes:

  • There is a shorthand way to add plugins. pluginXyz: netlify-plugin-example
  • type property can/might still have another name
  • See Reserved top level keys. Those would be for core usage. See research for other things we may need
  • Loading same plugin twice but with older version is example of loading 2 different versions of a plugin. Unclear on how that works with npm installs.

Looks kind of messy with all the comments. It's cleaner without. This format also plays nicer with toml land

# Netlify plugins
plugins:
  # shorthand for plugin with no options. LogicalName: plugin-name
  pluginXyz: netlify-plugin-example
  # expanded config
  pluginOne:
    type: ./plugins/plugin-one
  # Reserved top level keys
  pluginTwo:
    type: npm-package-name
    # is plugin on or off
    enabled: true
    # max time allowed for plugin to run
    timeout: 5m
    # env vars set inside this plugin only
    env:
      thing: 'only accessible via process.env.thing in this plugin'
    # secrets (maybe just array of keys)
    secrets:
      shhh: 'only accessible via this plugin'
  # Loading same plugin twice but with older version
  pluginTwoOlderVersion:
    type: [email protected]
  # expanded config with plugin configuration
  customFunctions:
    type: netlify-plugin-functions
    enabled: true
    functions:
      foo:
        handler: src/backend/whatever.js
        path: /api/lol
        method: POST
  # cypress integration testing
  CypressPlugin:
    type: netlify-plugin-cypress
    baseUrl: ${env:URL, 'https://netlify.com'}
    pageLoadTimeout: 60000
    arrayOfValues:
      - lol
      - hi
  # sms on builds
  NotifierPlugin:
    type: netlify-plugin-notifier
    enabled: false
    sms:
      provider: twilio
      TWILIO_ACCOUNT_SID: ${env:TWILIO_ACCOUNT_SID, 'NA'}
      TWILIO_AUTH_TOKEN: ${env:TWILIO_AUTH_TOKEN, 'NA'}
      TWILIO_PHONE_NUMBER: ${env:TWILIO_PHONE_NUMBER, 'NA'}
  # get lighthouse score between builds
  lightHouse:
    type: netlify-plugin-lighthouse
    enabled: ${env:LIGHTHOUSE_ENABLED, false}
    currentVersion: 0.0.4
    compareWithVersion: 0.0.3
  # a11y testing
  AxePlugin:
    type: netlify-plugin-axe
    axeFlags: '--rules color-contrast,html-has-lang'
    enabled: false

The reserved top level keys might grow as time passes and new use cases arise. This might cause key clashes with user land plugin config. For this reason, it might make sense for all plugin configuration to be defined 1 level lower. pluginLogicalID.config.foo

Example:

# Netlify plugins
plugins:
  customFunctions:
    type: netlify-plugin-functions
    enabled: true
    config:
      functions:
        foo:
          handler: src/backend/whatever.js
          path: /api/lol
          method: POST

from build.

ehmicky avatar ehmicky commented on May 18, 2024

Thanks, we're getting there! :D

To summarize your example:

plugins:
  pluginId: pluginLocation
  pluginId:
    type: pluginLocation
    enabled: boolean
    // other reserved keys
    config: object

Some follow-up questions:

  • 👍 on namespacing plugin configuration. Should it be called options instead of config? I am wondering whether config would be confusing since the netlify.yml is a config as well.
  • What about omitting (or not) netlify-plugin-* from package names (see my questions above)?
  • How to distinguish plugins that need to run serially and in parallel?

from build.

DavidWells avatar DavidWells commented on May 18, 2024

Should it be called options instead of config?

Need to think on that one 🤯

What about omitting (or not) netlify-plugin-* from package names (see my questions above)?

Omitting the name is confusing. I never really liked seeing that in babel/eslint world. Made things harder to find. (having to know about a hidden convention)

Explicitly typing the full value should be required. More characters but more readable / understandable

How to distinguish plugins that need to run serially and in parallel?

Good question. This should probably be something that plugin authors can set and maybe users override.

Below are 2 ideas. skip to disable a given method and async (or parallel) to force lifecycle methods to not block the rest of the chain

plugins:
  pluginId: pluginLocation
  pluginId:
    type: pluginLocation
    # user defined "run this in parallel. Could have different key name
    async: ['init'] 
    # user option to skip a given lifecycle step if they dont need it or want to disable 
    skip: ['postbuild']
    # plugin conf
    config: object

from build.

ehmicky avatar ehmicky commented on May 18, 2024

Omitting the name is confusing. I never really liked seeing that in babel/eslint world. Made things harder to find. (having to know about a hidden convention)

👍

Below are 2 ideas. skip to disable a given method and async (or parallel) to force lifecycle methods to not block the rest of the chain

If there was a way to make serial/parallel orchestration transparent to the user, this might provide with a better developer experience?

I am thinking parallelism might need a separate issue, so I created #168 for this question.

from build.

sdras avatar sdras commented on May 18, 2024

Mentioned this to @DavidWells in slack as he messaged me for feedback:

  • Personally I like it, I find it to be more explicit
  • There are die-hard functional people who will (maybe) not like it and want an array instead 🤷🏻‍♀️

from build.

DavidWells avatar DavidWells commented on May 18, 2024

Here is the updated plugin syntax based on discussions here and in other threads (re parallelization)

plugins:
  pluginLogicalId:
    # type of plugin.
    type: pluginLocation # Points to node_modules or local relativePath ./path
    # plugin config
    config:
      foo: bar
      baz: fizz
      xyz: ${otherPlugin.outputs.MY_KEY} # dependsOn otherPlugin to run first
    ########################
    ##  Advanced Options  ##
    ########################
    # Disable plugin completely
    enabled: boolean # default true (typically omitted by user)
    # max time allowed for plugin to run
    timeout: 5m # format 20s, 3m
    # Force lifecycle method to run in parralel
    parallel: # array or `true` for all
      - init
    # Skip a given lifecycle step from plugin
    skip: # array or `true` for all
      - postbuild
      - finally
    ########################
    ##  FUTURE Options    ##  (don't implement yet needs more though)
    ########################
    # If error thrown, ignore & don't hard fail build
    ignoreErrors: false
    # retry lifecycle methods if failed. maxAttempts/delay
    retry: 3
    # secrets by key
    secrets: # accessible by this
      - MY_SECRET_KEY

from build.

ehmicky avatar ehmicky commented on May 18, 2024

I have broken down this issue into the following issues: #188, #189, #190, #191, #192, #193.

from build.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.