feat: implement plugin wrappers #1332

tomzemp · 2023-02-20T09:39:10Z

Implements LIBS-397
Note: this PR is related to corresponding app-platform PR. The PluginSender wrapper here will only be of use if building a plugin with the changes in the app-platform PR.

Key features

Adds services/plugin, particularly PluginSender component that allows one to define a plugin and pass props to it (sets up part of communication bridge via post-robot)
modifies services/alerts to allow for hoisting of alerts from child iframe

Checklist

Have written Documentation
- delaying writing documentation until we have decided overall approach
Has tests coverage
- did not write tests until we have decided overall approach

Known Issues

the retrieval of the appropriate plugin entrypoint is dependent on api/apps being expanded to include bundled apps (https://dhis2.atlassian.net/browse/DHIS2-7154) and to filter out apps that are not accessible to a user (https://dhis2.atlassian.net/browse/DHIS2-14750)
Error boundaries (in this case, error boundary for non-accessible/non-installed apps) still need to be designed/implemented
I just put any for types while writing this code and will need to update those

tomzemp · 2023-02-20T09:40:29Z

services/alerts/src/useAlert.ts

-                },
-                alertRef
-            )
+            if (plugin && parentAlertsAdd && !showAlertsInPlugin) {


I passed this showAlertsInPlugin down to useAlert as I thought it made the logic clearer here, but it could be incorporated into logic further up and then not passed down.

tomzemp · 2023-02-20T09:42:42Z

services/alerts/src/index.ts

@@ -1,3 +1,4 @@
 export { AlertsProvider } from './AlertsProvider'
 export { useAlerts } from './useAlerts'
 export { useAlert } from './useAlert'
+export { AlertsManagerContext } from './AlertsManagerContext'


this is now being exported so that the PluginSender can access add from the parent (app) and pass it to the plugin, so that the alerts can then be added to the app's AlertsManager rather than the plugin's

tomzemp · 2023-02-20T09:47:08Z

services/plugin/src/PluginSender.tsx

+
+    if (data && !pluginEntryPoint) {
+        return (
+            <PluginError


if the pluginEntryPoint is missing the app is not installed (when api/apps is updated to exclude apps to which user does not have access to, this could also mean app is not accessible). This error needs to be in the app. A disadvantage of having it in app-runtime is that d2-i18n does not work in typescript currently, so we need to update to provide some basic translations (I think like our current app error boundary, just translating to French and Spanish might be enough?)

HendrikThePendric

General approach is looking good. I can review again once this comes out of draft. One small thing that did stand out to me is some of the variable naming with parent in it. I suppose this relates to the window.parent, but I feel like this name is a bit too generic and possibly a bit too closely tied to the technical implementation of the solution. To be a bit more specific:

If you'd want to stick to a name that is tied to the technical implementation, I think it would be wise to use a more specific name such as parentWindow or iframeParent, to signify that we're not just dealing with a DOM element's parent, but a parent window.
Alternatively, I think a name which is more tied to the features we've got could also make sense. We're dealing with an app which can host plugins, so perhaps we can simply swap the parent prefix/suffix in variable names with app. For example parentAlertsAdd could be replaced by appAlertsAdd.

I was also considering before (but forgot to discuss this with you) that we could consider borrowing description from the virtualisation world (as in Virtual Machines). There words guest and host seem quite apt. For example, the PluginWrapper could be renamed to PluginHost and the component that wraps the plugin within the iframe could be called the PluginGuest. I think it's possibly a bit late to change all of that, but thought I'd mention it anyway.

HendrikThePendric · 2023-02-23T09:11:56Z

services/plugin/src/PluginSender.tsx

+    const [inErrorState, setInErrorState] = useState<boolean>(false)
+
+    useEffect(() => {
+        if (iframeRef?.current) {


Doing an early return would avoid everything being indented

KaiVandivier

Approving to allow for merge to alpha -- reviewing can continue there

# [3.10.0-alpha.1](v3.9.0...v3.10.0-alpha.1) (2023-03-13) ### Bug Fixes * add documentation, clean up ([c537590](c537590)) * clean up, add useless test ([b14952b](b14952b)) * custom error handling ([c72fc6e](c72fc6e)) * dependency resolution ([2480c1c](2480c1c)) ### Features * ideas for plugin wrappers [LIBS-397] ([be38607](be38607)) * implement plugin wrappers (alpha) ([#1332](#1332)) ([56a9a3f](56a9a3f)) * plugin wrappers, errors + alerts ([bda6a43](bda6a43)) * update plugin wrappers ([30c963c](30c963c))

dhis2-bot · 2023-03-13T15:34:07Z

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

The release is available on:

Your semantic-release bot 📦🚀

# [3.11.0-alpha.1](v3.10.1...v3.11.0-alpha.1) (2023-12-21) ### Bug Fixes * add back plugin service dependency [LIBS-583] ([ca10691](ca10691)) * add back plugin service dependency [LIBS-583] ([6d43ae3](6d43ae3)) * add documentation, clean up ([c537590](c537590)) * add in plugin service in runtime package ([#1343](#1343)) ([ed06a9f](ed06a9f)) * add width to plugin documentation [LIBS-487] ([b2c6273](b2c6273)) * check memomized props for postMessage communication [LIBS-514] ([b1a3a0a](b1a3a0a)) * clean up ([e53ecbd](e53ecbd)) * clean up, add useless test ([b14952b](b14952b)) * custom error handling ([c72fc6e](c72fc6e)) * dependency array ([03ce64f](03ce64f)) * dependency resolution ([2480c1c](2480c1c)) * merge issues ([496472a](496472a)) * move eslint disable line ([48912d7](48912d7)) * plugin experimental docs ([be215b2](be215b2)) * prevent sending updated props to plugin when props do not change [LIBS-514] ([86c6f75](86c6f75)) * reset communication on either pluginSource or pluginShortName change ([3fdae5b](3fdae5b)) * temporarily disable failing test ([6664199](6664199)) * trigger props resend when iframe src changes [LIBS-488] ([f4a6680](f4a6680)) * trigger props resend when iframe src changes [LIBS-488] [#1344](#1344) ([cea7600](cea7600)) * type error ([9c17206](9c17206)) * update alpha branch [skip release] ([ccb793c](ccb793c)) * working autorsize width ([2991045](2991045)) ### Features * add autoresizing for height ([dbb6e26](dbb6e26)) * experimental plugin release ([f5cca86](f5cca86)) * ideas for plugin wrappers [LIBS-397] ([be38607](be38607)) * implement plugin wrappers (alpha) ([#1332](#1332)) ([56a9a3f](56a9a3f)) * plugin experimental export ([25f02a6](25f02a6)) * plugin wrappers, errors + alerts ([bda6a43](bda6a43)) * update plugin wrappers ([30c963c](30c963c))

dhis2-bot · 2023-12-21T08:55:06Z

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

The release is available on:

Your semantic-release bot 📦🚀

tomzemp added 8 commits December 21, 2022 10:15

feat: ideas for plugin wrappers [LIBS-397]

be38607

feat: update plugin wrappers

30c963c

fix: clean up, add useless test

b14952b

chore: start pluginwrapper error boundary

46c9219

chore: sample error

ddc4251

fix: custom error handling

c72fc6e

feat: plugin wrappers, errors + alerts

bda6a43

chore: move plugin wrapper logic to app-platform

1681f6e

tomzemp commented Feb 20, 2023

View reviewed changes

tomzemp marked this pull request as draft February 20, 2023 09:50

tomzemp mentioned this pull request Feb 20, 2023

feat: implement plugin wrapper (receiver in plugin) dhis2/app-platform#786

Merged

HendrikThePendric reviewed Feb 23, 2023

View reviewed changes

fix: add documentation, clean up

c537590

tomzemp changed the base branch from master to alpha March 9, 2023 12:02

tomzemp requested a review from KaiVandivier March 9, 2023 12:03

tomzemp marked this pull request as ready for review March 9, 2023 15:34

KaiVandivier approved these changes Mar 9, 2023

View reviewed changes

tomzemp changed the base branch from alpha to master March 13, 2023 13:38

tomzemp changed the base branch from master to alpha March 13, 2023 13:38

Merge branch 'alpha' into LIBS-397/plugin-wrappers-sender

41124e8

tomzemp force-pushed the LIBS-397/plugin-wrappers-sender branch from 8489a2b to 2480c1c Compare March 13, 2023 14:43

fix: dependency resolution

2480c1c

tomzemp merged commit 56a9a3f into alpha Mar 13, 2023

tomzemp deleted the LIBS-397/plugin-wrappers-sender branch March 13, 2023 15:18

dhis2-bot added the released on @alpha label Mar 13, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: implement plugin wrappers #1332

feat: implement plugin wrappers #1332

tomzemp commented Feb 20, 2023 •

edited

Loading

tomzemp Feb 20, 2023

tomzemp Feb 20, 2023

tomzemp Feb 20, 2023

HendrikThePendric left a comment

HendrikThePendric Feb 23, 2023

KaiVandivier left a comment

dhis2-bot commented Mar 13, 2023

dhis2-bot commented Dec 21, 2023

feat: implement plugin wrappers #1332

feat: implement plugin wrappers #1332

Conversation

tomzemp commented Feb 20, 2023 • edited Loading

Key features

Checklist

Known Issues

tomzemp Feb 20, 2023

Choose a reason for hiding this comment

tomzemp Feb 20, 2023

Choose a reason for hiding this comment

tomzemp Feb 20, 2023

Choose a reason for hiding this comment

HendrikThePendric left a comment

Choose a reason for hiding this comment

HendrikThePendric Feb 23, 2023

Choose a reason for hiding this comment

KaiVandivier left a comment

Choose a reason for hiding this comment

dhis2-bot commented Mar 13, 2023

dhis2-bot commented Dec 21, 2023

tomzemp commented Feb 20, 2023 •

edited

Loading