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

feat: implement plugin wrapper (receiver in plugin) #786

Merged
merged 7 commits into from
Mar 13, 2023

Conversation

tomzemp
Copy link
Member

@tomzemp tomzemp commented Feb 20, 2023

This PR is conceived in conjunction with PR in app-runtime to provide plugin wrappers.

The main change in this code is adding PluginLoader in the app shell which sets up communication with the parent window. For the communication to work, PluginSender needs to be used in the app, otherwise, the communication will fail (the plugin would still load, so you can still build a plugin with this updated app shell and not use PluginSender if you do not want this communication channel, but do want a separate plugin build)

There is some additional updates in app/adapter for hoisting plugin alerts and errors.

Some notes

  • Error boundaries are to be designed by Joe. For now, I have just modified the existing Error Boundary to make it work with a plugin
  • I'm not really sure what we want to do with checks for required props. Previously when the wrapper within the plugin was in app-runtime, I set it up to take a requiredProps parameter. Now I've set it up to read requiredProps from d2.config.js (which I do not like from a practical perspective as it is now putting these into an environment variable (string) and reparsing). I feel like, it would be better to just let people check for required props in the plugin rather than trying to incorporate that as a default process into the build logic of the plugin. If we want this to be easier, we could create a custom hook in app-runtime/services/plugin that lets you pass your required props and then will throw an error if they are missing?

@tomzemp tomzemp marked this pull request as draft February 20, 2023 10:04

const Plugin = ({ config }) => {
return (
<PluginOuterErrorBoundary>
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this exists, so that an error in PluginLoader could be caught. I'm not really sure that this is ultimately necessary* as most of the logic there is just to handle the post-robot communication which is set to silently fail, so as not to impede plugin's usability if communication is not needed. If we do want an error boundary here, I guess we could reuse the same one from app-platform/adapter?

  • right now, it's necessary based on how required props are being handled, but I think this should probably change

@tomzemp tomzemp changed the base branch from master to alpha March 9, 2023 10:21
@tomzemp tomzemp marked this pull request as ready for review March 9, 2023 12:02
@tomzemp tomzemp merged commit d4f1ee2 into alpha Mar 13, 2023
@tomzemp tomzemp deleted the LIBS-397/plugin-wrappers-platform branch March 13, 2023 15:17
dhis2-bot added a commit that referenced this pull request Mar 13, 2023
# [10.4.0-alpha.1](v10.3.1...v10.4.0-alpha.1) (2023-03-13)

### Bug Fixes

* merge in master branch of app-platform ([5c637c0](5c637c0))
* pass props with spread operator ([bd4dccb](bd4dccb))
* simplify error reset logic ([d40dfba](d40dfba))
* style adapter package file ([d5e17e1](d5e17e1))

### Features

* implement plugin wrapper (receiver in plugin) (alpha) ([#786](#786)) ([d4f1ee2](d4f1ee2))
* plugin error handling ([7fd0605](7fd0605))
* plugin handling ([7ee8ed6](7ee8ed6))
@dhis2-bot
Copy link
Contributor

🎉 This PR is included in version 10.4.0-alpha.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants