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.

Sign up for GitHub

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

docs: Add ADR for handling frontend plugin fallback #541

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
+38 −0
Open

docs: Add ADR for handling frontend plugin fallback #541

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
905fe76
docs: Add ADR for handling frontend plugin fallback
jsnwesson Aug 18, 2023
4c78b7f
Update docs/decisions/0008-frontend-plugin-fallback.rst
jsnwesson Sep 27, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 38 additions & 0 deletions docs/decisions/0008-frontend-plugin-fallback.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
How to Handle Frontend iFrame Plugin Fallback
======================================================================================

Status
------

Draft

Context
-------
The Frontend Plugin Framework's iFrame class implementation will live in Frontend Platform,
but the question of how we want to handle the case where the iFrame fails to render needs to
handled.

Given that this is a new feature that is being made available to the frontend, we
can't anticipate all of the ways that the plugin will be used for a given repo, nor do we
have any intention to limit the scope of how it can be used. Because we don't all of the use
jsnwesson marked this conversation as resolved.
Show resolved Hide resolved
cases for the plugin, we also don't know the desired fallback method that is desired for any
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
cases for the plugin, we also don't know the desired fallback method that is desired for any
cases for the plugin, we also don't know the desired fallback method for any

given plugin.

Decision
--------
For the above reasons, we will have failing iFrame plugins return an H1 tag that
Copy link
Author

Choose a reason for hiding this comment

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

I'm fully aware that this is likely not the best way to deliver an error, but in hopes of stirring enough of the pot to bring people into the conversation, I'll leave it here (see Constructive Controversy)

Copy link
Contributor

Choose a reason for hiding this comment

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

Can we log this in a way that the teams can determine how often this fails for their host site?

Copy link

Choose a reason for hiding this comment

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

How will the host MFE know that the iFrame has failed to render?

Copy link
Member

Choose a reason for hiding this comment

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

[curious] Is there an opportunity to use an ErrorBoundary in some way here? Either the one defined/used by @edx/frontend-platform or a custom one for this use case.

Copy link
Author

Choose a reason for hiding this comment

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

Ah! You know, I should update this to name that there is a PluginErrorBoundary component that is part of the PR I'm working on for this.

In short, the PluginErrorBoundary will capture any errors that occur within, and whatever decision is made here on how to best reflect an error will be handled by the error boundary.

Copy link
Author

Choose a reason for hiding this comment

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

https://github.com/openedx/frontend-platform/pull/235/files#diff-a00a6c55618b4e46a1c5b58c4fa2d25c636df8f5fa5e5011a0612ad14a82c887

says "There was an error.". By doing so, it is up to the engineer/team that implements the plugin
to handle the desired fallback however they would like.
Comment on lines +24 to +25
Copy link
Contributor

Choose a reason for hiding this comment

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

How will we do that? By checking for the element returned by the pluginSlot?

Copy link

Choose a reason for hiding this comment

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

How we will be able to check the element returned by the pluginSlot?

Copy link
Author

Choose a reason for hiding this comment

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

hey all! huge apologies for the delay on these responses. After having written this ADR draft, I realized that the PR I was taking on wasn't in any state for me to confidently know what worked and thus a lack of confidence in the ways we could handle the fallback.

I will be editing this draft to reflect my findings and we'll ideally have a clearer picture to work off of.


Rejected Alternatives
---------------------

The initial assumption was that we should determine what the fallback would be
if the iFrame plugin render failed. The extent of what this handling could look like
ranges from taking in a fallback argument to providing the same fallback image no matter
how the plugin was being used. These options limit the possibilities for how the fallback
-- and the plugin itself -- would present itself by assuming each plugin implementation
or desired fallback method will be the same for all plugins.

.. _oepXXXX: https://github.com/openedx/open-edx-proposals/pull/287
.. _frontend_plugin_roadmap: https://github.com/openedx/platform-roadmap/issues/27