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

draft proposal: new extension points system #2032

Closed
rbuels opened this issue Jun 2, 2021 · 5 comments · Fixed by #2140
Closed

draft proposal: new extension points system #2032

rbuels opened this issue Jun 2, 2021 · 5 comments · Fixed by #2140
Labels
enhancement New feature or request

Comments

@rbuels
Copy link
Contributor

rbuels commented Jun 2, 2021

class MyPluginWithExtensionPoints extends Plugin {

  configure(pluginManager) {
    pluginManager.createExtensionPoint('MyPluginWithExtensionPoints.menuItems', [ defaultMenuItems ])
  }
}
const SomeReactComponent = (props) => {
   const pluginManager; // from someplace

  const composedMenuItems = pluginManager.evaluateExtensionPoint('MyPluginWithExtensionPoints.menuItems')
}
class PluginManager {
   evaluateExtensionPoint(epName:string, defaultArgs: unknown) {
      const callbacks: Function[] = this.extensionPoints[epName]

      // if an ep had functions: [ one, two, three ], meaning that "callbacks" above
      // this would return three(two(one(defaultArgs)))
    }
}
class SomeOtherPlugin extends Plugin {
   configure(pluginManager) {
     pluginManager.addToExtensionPoint('MyPluginWithExtensionPoints.menuItems', (otherMenuItems) => {
         return [ ...otherMenuItems, { mycrazymenuitem } ]
     })
   }
}
@rbuels rbuels added enhancement New feature or request discuss in meeting labels Jun 2, 2021
@rbuels
Copy link
Contributor Author

rbuels commented Jun 2, 2021

might streamline the use case talked about in #2006

@garrettjstevens
Copy link
Collaborator

I had another idea for extending plugins. It's more similar to the autorun solution we have for plugins adding custom right click menu options here:

autorun(() => {
const session = pluginManager.rootModel?.session as Session | undefined
if (session) {
session.views.forEach(view => {
if (view.views) {
view.views.forEach(v => addContextMenu(v))
} else {
addContextMenu(view)
}
})
}
})

The idea is that instead of having to have additionalContextMenuItemCallbacks and addAdditionalContextMenuItemCallback(), we could use the simulated inheritance described here to inherit contextMenuItems. We would have to make contextMenuItems be non-getter view. Then in our pluggable types (e.g. DisplayType) we would do something like this:

export default class DisplayType extends PluggableElementBase {
  // the stuff that's already in DisplayType...

  extendStateModel(extension) {
    this.stateModel = this.stateModel.extend(extension)
  }
}

And then in the plugin's configure you would do something like:

class SomePlugin extends Plugin {
  configure(pluginManager) {
    const displayType = pluginManager.getDisplayType('LinearAlignmentsDisplay')
    displayType.extendStateModel(self => {
      const superContextMenuItems = self.contextMenuItems
      return {
        views: {
          contextMenuItems() {
            const menuItems = superContextMenuItems()
            menuItems.push(/* new menu items here */)
            return menuItems
          },
        },
        actions: {
          // could add actions, too
        },
      }
    })
  }
}

This is a bit less general than the generic extension points, since you can only extend things in a plugin's stateModel, but I think addresses the use cases we've talked about so far.

@rbuels
Copy link
Contributor Author

rbuels commented Jun 3, 2021

that's a neat idea. it's nice that it doesn't really introduce any new architecture.

@cmdcolin
Copy link
Collaborator

cmdcolin commented Jun 3, 2021

that could be ok. I tried using both extend and just compose and it appeared to operate similarly...continuing on my testing codesandbox from mobx-state-tree https://codesandbox.io/s/blue-dawn-wrgvj?file=/src/App.js

@cmdcolin
Copy link
Collaborator

cmdcolin commented Jun 3, 2021

we may have to think about what things need to be changed into functions to make this work.. I envision renderProps may also need to change to a function, and really it sort of boils down to anything we forsee being derived...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
3 participants