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

[RFC] Plugins v2.0 #919

Closed
wants to merge 1 commit into from
Closed

Conversation

brandon93s
Copy link
Contributor

@brandon93s brandon93s commented Mar 1, 2018

Note: This is in the very early stages of development still. I'm opening this to begin a discussion on the direction & approach before proceeding any further. There is still a ton of work to be done if we want to peruse this. We need to consider any maintenance overhead this could introduce.


Details

What: This is a complete plugin system overhaul. The proposed implementation gives the plugin developer full-reign to hook into any part of parcel that they see fit. Existing plugins remain compatible via a shim in the new system, but the exported API of parcel would change for programmatic consumers. We could likely maintain the existing exported API - but without plugin support for a few versions to ease transition.

Highlights:

  • Multiple plugins per file / feature / via class extension (not replacement). fixes multi plugin for same file only execute one  #902
  • Plugins can tap into any part of Parcel
  • A single plugin can tap into multiple parts of Parcel
  • Existing plugins will need to stick around to register new assets. The code currently calls these "legacy" but in reality they probably have a necessary place long-term.
  • Core classes remain unaltered when no plugins are installed

So Far: The code within demonstrates an approach to plugins that allows plugin developers to "mixin" their functionality. For now, the approach has been added to the Bundler, Logger, and HTMLPackager strictly for demonstration purposes. You can extrapolate the pattern to see how it would be applied across the code base - a minor addition to each file we'd want to support.

Examples

Unicorn Logger

Plugin Implementation:

// parcel-plugin-unicorn-logger/index.js
module.exports.Logger = superclass => class extends superclass {
    _log(message) {
        super._log(`🦄 ${message}`)
    }
}

Demo:

image

HTML Comment Header

You could imagine that this plugin would ship configurable, so the user could configure their header. The following demonstrates a hard-coded comment header though.

Plugin Implementation:

// parcel-plugin-html-header/index.js
module.exports.HTMLPackager = superclass => class extends superclass {
    setup() {
        super.setup();
        const write = this.dest.write;
        
        this.dest.write = html => {
            return write(`<!-- Comment Injected By Plugin! --> \n\r ${html}`)
        }
    }
}

Demo:

image

Next Steps

  • Need to continue the implementation, expanding plugin application to remaining classes
  • Update unit tests to consume new API
  • Implement new tests - lots of tests!
  • Enhance the API to consume a plugin order. This would be passed into Plugin.init and would control the order that mixins are applies
  • Long term, document the classes ("API Docs") to encourage plugin development
  • Consider extracting necessary code from mixwith into a utility - it contains more than we need

#918 CC: @davidnagli @devongovett @DeMoorJasper

@brandon93s brandon93s added 💬 RFC Request For Comments 💡POC Proof Of Concept 📝 WIP Work In Progress labels Mar 1, 2018
@brandon93s
Copy link
Contributor Author

While powerful, my biggest hesitation to continue with this is with how fragile plugin implementations would be. They'd be relying on implementation details... not ideal. We'd have to be extra careful with our semantic versioning to allow plugin developers to properly lock their peerDependency.

This may be more gimmicky than realistic.

@devongovett
Copy link
Member

devongovett commented Mar 1, 2018

Cool, thanks for starting to think about this! I think we should follow two principles:

  1. The parcel core should be written the same way as plugins so people can have examples to follow when implementing plugins. Also, with the number of asset types growing in parcel core, I think we will eventually want to split these out into separate packages anyway - which would all be written like plugins.
  2. We should favor composition over inheritance. Inheriting classes the way we do now for assets is dangerous and prone to reliance on implementation details. We should come up with a way for asset types to compose together in pipelines rather than relying on a single asset class per file type being applied as we do currently.

I think these two principles can guide us in the right direction. I know @jamiebuilds has been doing some thinking in this area as well. Would be good to hear from him as well (perhaps in a separate RFC?).

@jamiebuilds
Copy link
Member

Some more guiding principles:

  • There shouldn't be one "plugin" type to do everything. You lose design flexibility and it only causes confusion. We should have parcel-asset-*, parcel-reporter-*, etc.
  • "plugins" should not have access to Parcel internals, ever. To an asset, nothing should be "observable" other than it's been called with some input and output ("pure").
  • Plugins should have no awareness of one another. They should not be able to modify one another
  • Plugins shouldn't be allowed to hold onto any state. It should not be a class, and the same plugin should be able to run in separate processes without any intercommunication.
  • Asset plugins should not be able to control things outside of the current file's transformation

Ideally the shape of a plugin should fit within this:

type JSONValue =
  | null
  | boolean
  | number
  | string
  | Array<JSONValue>
  | JSONObject;

type JSONObject = {
  [key: string]: JSONValue,
};

type Plugin = {
  [key: string]: (input: JSONObject) => JSONObject,
};

declare module.exports: Plugin;
  • module.exports is a Plugin
  • Plugin is an object with methods
  • The methods all accept one argument which is a JSON-serializable object
  • The methods all return an object which is JSON-serializable

Additionally:

  • Methods should be run in a well defined life-cycle
  • Decisions should always be returned from a method, they should not call out to other APIs
  • Methods should be able to pass information from one step to another through the JSON-serializable value:
module.exports = {
  methodOne(input) {
    return { ..., extra: { foo: true } };
  },
  methodTwo(input) {
    console.log(input.extra.foo); // true
  }
};

Non-requirements

  • It would be really nice if the asset plugin API did not import parcel-bundler, it should ideally have no need to
  • The plugin API should be spec-ed out in great detail so other implementations can exist

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
📝 WIP Work In Progress 💡POC Proof Of Concept 💬 RFC Request For Comments
Projects
None yet
Development

Successfully merging this pull request may close these issues.

multi plugin for same file only execute one
3 participants