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

New API for external plugins #1976

Open
saschanaz opened this issue Dec 26, 2018 · 6 comments · May be fixed by #3320
Open

New API for external plugins #1976

saschanaz opened this issue Dec 26, 2018 · 6 comments · May be fixed by #3320
Projects

Comments

@saschanaz
Copy link
Member

saschanaz commented Dec 26, 2018

The current plugin API depends on AMD module system (via requirejs) and exposes internal modules. This blocks #1975 and causes potential breaking changes when any internal functions change.

We may introduce a new API that looks something like webpack one:

var respecConfig = {
   // ...
   plugins: [
     new MyPlugin()
   ]
};

@sidvishnoi @marcoscaceres Thoughts?

@saschanaz
Copy link
Member Author

Looks like the vast majority of plugin users only depends on core/pubhubsub.

@saschanaz
Copy link
Member Author

Maybe we can set window.require that supports certain modules to keep backward compatibility. For example:

if (!window.require) {
  window.require = function(deps, callback) {
    const modules = deps.map(dep => {
      if (!(dep in window.require.modules)) {
        throw new Error("Invalid dependency name");
      }
      return window.require.modules[dep];
    });
    callback(modules);
  };
  window.require.modules = {};
}
// inside modules e.g. pubsubhub
if (window.require && window.require.modules) {
  window.require.modules["core/pubsubhub"] = { sub };
}

Will probably be longer than this, though... @sidvishnoi, does this idea look okay?

@sidvishnoi
Copy link
Member

That's a really cool idea. We won't need to bundle requireJS this way.

@saschanaz
Copy link
Member Author

window.require thing landed via #1981

@sidvishnoi
Copy link
Member

sidvishnoi commented Jul 16, 2020

I'm not sure we want to support "tapping" into any/all core plugin/hook. How about something like:

const myPlugin = {
  name: 'plugin-name',
  // some custom hook name instead of listening to pubsub events, or, "when": "after-markdown", "when": "before-markdown"
  after: 'markdown',
  // conf is respecConfig as usual.
  // utils is an object of methods as an abstraction over respec internals, 
  //  making us expose as little as required, reducing breaking changes in future.
  async run(conf, utils) {
    utils.showError(`${conf.unCool} option has invalid value.`)
  },
};

This follows pretty much same structure as present ReSpec plugins.

core/base-runner can do a pass where it collects the run order for all plugins in respecConfig.plugins[], and modifies the overall modules array. Using a plain object as plugin helps in keeping it simple/clean.
Internally, we might expose pubsub events to running plugins. Note that it's better to allow hooks only before/after plugin running, so they're not subject to changes in internals of core plugins and only depend on plugin order, which we limit by exposing a few hooks.

@saschanaz
Copy link
Member Author

It looks promising! Probably needs some experiment by converting existing specs to make sure that we have all required features.

@sidvishnoi sidvishnoi added this to Prioritized in Up next Aug 24, 2020
@sidvishnoi sidvishnoi mentioned this issue Sep 3, 2020
6 tasks
@sidvishnoi sidvishnoi linked a pull request Feb 19, 2021 that will close this issue
3 tasks
@sidvishnoi sidvishnoi moved this from Prioritized to In progress in Up next Feb 23, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Up next
  
In progress
Development

Successfully merging a pull request may close this issue.

2 participants