-
Notifications
You must be signed in to change notification settings - Fork 64
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
docs: Add ADR for handling frontend plugin fallback #541
base: master
Are you sure you want to change the base?
Conversation
Codecov ReportAll modified lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #541 +/- ##
==========================================
+ Coverage 83.05% 83.17% +0.12%
==========================================
Files 40 40
Lines 1062 1070 +8
Branches 195 195
==========================================
+ Hits 882 890 +8
Misses 168 168
Partials 12 12 ☔ View full report in Codecov by Sentry. |
|
||
Decision | ||
-------- | ||
For the above reasons, we will have failing iFrame plugins return an H1 tag that |
There was a problem hiding this comment.
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)
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. |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
|
||
Decision | ||
-------- | ||
For the above reasons, we will have failing iFrame plugins return an H1 tag that |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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?
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 | ||
cases for the plugin, we also don't know the desired fallback method that is desired for any |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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 |
|
||
Decision | ||
-------- | ||
For the above reasons, we will have failing iFrame plugins return an H1 tag that |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Co-authored-by: Zainab Amir <[email protected]>
Hey, @jsnwesson, could you please pass the failed ci check? |
Description:
This ADR is to propose how we should handle the scenario where the iFrame plugin fails to render.
The decision here will help with releasing the PR to make iFrame plugins possible in MFEs. (#235)