docs: Add info on how to use frontend plugin slots #233
Conversation
arbrandes
changed the title
arbrandes
force-pushed
the
using-frontend-plugin-slots
branch
from
6f829e0 to
0016e7a
Compare
It's now possible to configure frontend plugin slots across MFEs (notably, header and footer). This documents how this can be done as simply as possible. See overhangio#199
arbrandes
force-pushed
the
using-frontend-plugin-slots
branch
from
0016e7a to
016915f
Compare
| /* We can't just assume FPF exists, as it's not declared as a dependency in all | ||
| * MFEs. Therefore, we have to use dynamic imports to check if it's available. | ||
| * | ||
| * We also have to use a `try` block (as opposed to `.then()`), otherwise | ||
| * Webpack won't treat the import as optional. Hence the async function. | ||
| */ |
There was a problem hiding this comment.
Note to reviewer: this particular workaround will no longer be necessary once all supported MFEs have at least one defined slot, forcing them to add frontend-plugin-framework to their list of dependencies.
There was a problem hiding this comment.
And even more so with frontend-base - which will include/replace FPF
There was a problem hiding this comment.
@brian-smith-tcril , Thank you for the clarification on the role of frontend-base. To confirm my understanding for future development, frontend-base will function as a shell that eventually replaces the frontend-plugin-framework (FPF), as well as standard elements like the header and footer.
My anticipation is that, even with frontend-base in use, we will still have the flexibility to update the header and footer packages individually. This would mean that packages such as the header, footer, and brand could still be customized and maintained as custom packages while being included in frontend-base.
| { | ||
| /* Hide the default footer. */ | ||
| op: PLUGIN_OPERATIONS.Hide, | ||
| widgetId: 'default_contents', | ||
| }, | ||
| { | ||
| /* Insert a custom footer. */ | ||
| op: PLUGIN_OPERATIONS.Insert, | ||
| widget: { | ||
| id: 'custom_footer', | ||
| type: DIRECT_PLUGIN, | ||
| RenderWidget: () => ( | ||
| <h1 style={{ "{{textAlign: 'center'}}" }}>This is the footer.</h1> | ||
| ), | ||
| }, | ||
| }, |
There was a problem hiding this comment.
Note to reviewer: I considered adding env.config.jsx templates directly to tutor-mfe and then exposing a new set of patches that would reduce some of the boilerplate, but the trade-off in flexibility for convenience seems to me to be unfavorable. To take full advantage of frontend plugins, particularly more complicated ones, users will need access to the whole env.config.jsx file anyway.
There was a problem hiding this comment.
Could you please clarify your approach here? I may not have fully grasped the patches thing and the trade-offs you're mentioning regarding flexibility and boilerplate reduction.
There was a problem hiding this comment.
@hinakhadim, sure, I replied more fully to Regis here: #233 (comment)
| # env.config.jsx files should live in `mfe/build/mfe`, which is also where the | ||
| # tutor-mfe Dockerfile goes. This is so it's more straightforward to copy it | ||
| # in, as seen below. |
There was a problem hiding this comment.
I realize it's iffy to recommend a plugin put a file directly into one of tutor-mfe's own directories, but for some reason I couldn't get the Dockerfile to COPY from a relative parent.
There was a problem hiding this comment.
In Indigo, I implemented a similar setup, but I have some concerns regarding the inclusion of env.config.jsx in mfe/build/mfe. Specifically, if we go this route, each user will need to take similar steps as I did in Indigo. This means they’ll either have to fork tutor-mfe and modify env.config.jsx or copy env.config.jsx from their custom plugin into mfe/build/mfe.
Can we think of any workaround to update or change env.config.jsx more seamlessly without requiring each user to fork plugin or override this file?
OR I am unsure that this env.config.jsx file will raise any issue for indigo plugin.
There was a problem hiding this comment.
Ah, I didn't look at how indigo does it. Let me check.
But yeah, the fact multiple plugins may want to modify env.config.jsx is not something I took into account, here.
| Using Frontend Plugin Slots | ||
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~ | ||
|
|
||
| The ``mfe-dockerfile-pre-npm-build`` patch is also suitable for taking advantage of the ``env.config.jsx`` mechanism, which lets you define and use frontend plugins. For instance, you can use it to modify the footer across all MFEs *without* having to maintain a fork of ``frontend-component-footer``. |
There was a problem hiding this comment.
Is there a link to some upstream documentation about env.config.jsx that we could use here?
There was a problem hiding this comment.
There's the ADR. I'll link to it, sure.
|
|
||
| The ``mfe-dockerfile-pre-npm-build`` patch is also suitable for taking advantage of the ``env.config.jsx`` mechanism, which lets you define and use frontend plugins. For instance, you can use it to modify the footer across all MFEs *without* having to maintain a fork of ``frontend-component-footer``. | ||
|
|
||
| Start by following the Tutor plugin tutorial at the `Adding new templates <https://docs.tutor.edly.io/tutorials/plugin.html#adding-new-templates>`_ section. Instead of adding a whole new ``Dockerfile``, though, you'll just modify the exiting one. Your ``myplugin.py`` should look something like this: |
There was a problem hiding this comment.
I see so many people adding double blank space after a dot: ". " Why is that? Is that some IDE feature?
There was a problem hiding this comment.
Do you mean after the period at the end of the sentence? Until very recently, this used to be the standard for typography (in English), up to and including word-processed text using a computer. A significant portion of people, myself included, were trained to do so.
There are discussions about why this was so, and it comes down to readability of monospaced text. If you're interested, the best write-up I can find is this repost of a since-lost blog post:
Anyway, I'm certainly not going to die on this hill. If you prefer the standard for the Tutor codebase to be one space, I ain't arguing. :)
| .. code-block:: javascript | ||
|
|
||
| /* We can't just assume FPF exists, as it's not declared as a dependency in all | ||
| * MFEs. Therefore, we have to use dynamic imports to check if it's available. |
There was a problem hiding this comment.
Really? Do you have a specific MFE in mind?
There was a problem hiding this comment.
The first build failure if you try to use the same env.config.jsx with FPF everywhere is frontend-app-authn. There might be others.
(This will all go away once we're in frontend-base land, or sooner, if all MFEs have slots.)
| let config = {}; | ||
|
|
||
| try { | ||
| const { DIRECT_PLUGIN, PLUGIN_OPERATIONS } = await import('@openedx/frontend-plugin-framework'); |
There was a problem hiding this comment.
If the FPF is not available, then we can't use plugins, right? So, could we write instead something like:
try {
const { DIRECT_PLUGIN, PLUGIN_OPERATIONS } = await import('@openedx/frontend-plugin-framework');
catch {
console.warn("⚠️ Frontend plugin framework unavailable in " + process.env.npm_package_name);
return;
}
(but with working code)
I'm a bit bothered by the very large try{...} catch{} statement.
There was a problem hiding this comment.
Sure, I'll try that - not sure how webpack is going to behave. Bundlers are weird with conditional imports.
I'll just object to the console warning in the browser, though. I'm usually against browser console logging in production code. There is often little reason why an end user should be able to see debug messages. A warning if something's broken, sure, but a learner doesn't need to know about FPF.
|
|
||
| As you can see, by using the ``process.env.npm_package_name`` variable you can use a single ``env.config.jsx`` to selectively apply configuration to different MFEs. | ||
|
|
||
| If, on the other hand, you prefer to maintain entirely separate ``env.config.jsx`` files, one for each mfe, that can also be done. For that you can rely on the ``mfe-dockerfile-pre-npm-build-*`` patches. For instance: |
There was a problem hiding this comment.
I don't think we should suggest an alternate solution here, it will just confuse people. I'd rather have a single, well-documented approach that everyone is aligned on.
There was a problem hiding this comment.
Ok, no objections!
| # env.config.jsx files should live in `mfe/build/mfe`, which is also where the | ||
| # tutor-mfe Dockerfile goes. This is so it's more straightforward to copy it | ||
| # in, as seen below. | ||
| hooks.Filters.ENV_TEMPLATE_TARGETS.add_item(("mfe/build/mfe", "plugins")) |
There was a problem hiding this comment.
I think that this approach is really confusing. Also, it doesn't allow multiple tutor plugins to add their own MFE plugins, which is a major limitation.
I suggest an alternative, which is to provide a default, no-op env.config.jsx file with all the right plumbing already in place and a {{ patch(...) }} statement for customization.
async function getConfig () {
let config = {};
try {
const { DIRECT_PLUGIN, PLUGIN_OPERATIONS } = await import('@openedx/frontend-plugin-framework');
config = {
pluginSlots: {}
}
return config;
}
function getPluginSlots(direct_plugin, plugin_operations) {
local slots = {};
{{ patch("mfe-plugin-slots") }}
return slots;
}
export default getConfig;
Then we would include information about how to implement the "mfe-plugin-slots" patch.
Would that work? Do you think it would be simpler to implement for plugin developers?
There was a problem hiding this comment.
Something like this was actually my original intention, so I'm not opposed off the bat. But let me pose the reasons I went with a generic env.config.jsx recommendation instead.
1. The tradeoff in convenience for flexibility isn't particularly favorable
The boiler plate, while a little arcane (mostly because of that try/catch, which is hopefully going away soon) is not that big if compared with the plugin definitions themselves. (Yes, I know, we should do something about the syntax. See the next point. ;) )
But if we don't expose the whole file, we're also limiting how people can use it. It is just javascript, and in theory you could use it to do cool things such as have getConfig() actually pull in configuration from an mfe_config-like micro-service somewhere. Or just a text file somewhere. (If you read through #199, this is actually where I think we should go with tutor-mfe after the frontend-base work lands, instead of using the mfe_config API.)
Counter-argument: the pre-npm-build patch will always exist. People that know what they're doing can do exactly what I proposed, with or without it being documented here.
2. Per-MFE configuration
Slots like the header and footer are present in more than one MFE, so there should be a way for the operator to configure them in a different way for each. However, frontend-plugin-framework does not provide a mechanism to do that.
In this PR I suggest a way to tackle this with a single env.config.jsx by leveraging javascript itself, giving the operator maximum control, which is the reason env.config.jsx exists in the first place. If instead we go with an mfe-plugin-slots patch - or, presumably, mfe-plugin-slots-* patches - we'll have to come up with some precedence logic that in all likelihood is going to be much more limited.
Counter-argument: So it's not just JSON. Are we now expecting operators to know Javascript? ;P
3. Plugin declaration is likely going to change over the next year
We'll have "true" plugins relatively soon, where operators just declare the plugins they want, not having to know or decide where components go on each page. (Maybe not by Teak, but likely Ulmo.) If we introduce a specific Tutor patch, this could mean an API change/deprecation in Tutor itself, too.
Counter-argument: frontend-base is going to require a lot of refactoring anyway, so we might as well make people's lives easier in the interim.
As you can see by the counter-arguments, it's ultimately a product decision. I'll let you decide. :) Now that I know a single env.config.jsx works, I'll be glad to modify the PR accordingly.
There was a problem hiding this comment.
I just realized I didn't address one major point you bring up that @hinakhadim hammered home afterwards. That multiple plugins may want to modify env.config.jsx. I have to think some more about this.
There was a problem hiding this comment.
I just went over Indigo's env.config.jsx, and I guess that's as good an example as any of my first point above. :)
The question I'm ruminating is: how do we let plugins do all that in a single file and still allow multiple plugins to coexist there?
Maybe the answer is that we should not allow plugin authors to run wild with env.config.jsx. Maybe all they can do is npm-install a plugin package via the post-npm-install patch, import the components via a new mfe-env-config-imports patch, then declare them via mfe-env-config-plugin-slots.
There was a problem hiding this comment.
Multiple plugins modifying env.config.jsx is definitely a tricky problem, and the fact that tutor-indigo already leverages it makes it quite the pressing issue.
The first thought that came to my mind was providing a way for plugins to "add their changes to the pile" so to speak. I tested the following env.config.jsx file
import { DIRECT_PLUGIN, PLUGIN_OPERATIONS } from '@openedx/frontend-plugin-framework';
const config = {
pluginSlots: {
desktop_header_slot: {
keepDefault: false,
plugins: [
{
op: PLUGIN_OPERATIONS.Insert,
widget: {
id: 'custom_header_component_one',
type: DIRECT_PLUGIN,
RenderWidget: () => (
<h1>1</h1>
),
},
},
]
},
desktop_header_slot: {
keepDefault: false,
plugins: [
{
op: PLUGIN_OPERATIONS.Insert,
widget: {
id: 'custom_header_component_two',
type: DIRECT_PLUGIN,
RenderWidget: () => (
<h1>2</h1>
),
},
},
]
},
},
}
export default config;and found that FPF handles it somewhat gracefully - the second component (the one that renders 2) is shown and the first component (the one that renders 1) is not.
In a world where each tutor plugin only touches one FPF slot this could be helpful - we could provide a way to "add changes to the pile" and have operators make sure the plugins are doing so in their preferred order. The obvious problem with that, however, is tutor plugins that utilize multiple FPF slots. If plugins A and B both replace the header and footer, and a site operator wants the header from A and footer from B this falls apart.
I'll keep thinking about this. It is definitely not a simple problem.
There was a problem hiding this comment.
Thanks for the productive discussion and @brian-smith-tcril for testing.
I’m currently facing a challenge with the scenario you mentioned: "If plugins A and B both replace the header and footer, and a site operator wants the header from A and the footer from B, this falls apart."
There was a similar instance where a user needed to use both tutor-indigo and a second plugin for additional customizations. From what I understand, the priority of each plugin’s components is determined by the order of activation. If tutor-indigo is enabled first, followed by the second plugin, the components from the second plugin will have priority.
...
RUN npm install @edx/frontend-component-header@npm:indigo-header
RUN npm install @edx/frontend-component-footer@npm:indigo-footer
RUN npm install @edx/frontend-component-header@npm: custom-header
RUN npm install @edx/frontend-component-footer@npm: custom-footer
...Conversely, if the second plugin is enabled first, tutor-indigo components will take precedence (means we use indigo header/footer in react js project).
...
RUN npm install @edx/frontend-component-header@npm: custom-header
RUN npm install @edx/frontend-component-footer@npm: custom-footer
RUN npm install @edx/frontend-component-header@npm: indigo-header
RUN npm install @edx/frontend-component-footer@npm: indigo-footer
...In cases where the operator needs the header from tutor-indigo (assuming that user cannot alter plugin) and the footer from the custom plugin (Plugin B), they would need to adjust Plugin B to exclude its header component, keeping only the footer. This way, Plugin A (Indigo) provides the header, and Plugin B supplies the footer without conflicts.
RUN npm install @edx/frontend-component-header@npm: indigo-header
RUN npm install @edx/frontend-component-footer@npm: indigo-footer
RUN npm install @edx/frontend-component-footer@npm: custom-footerAny further insights would be greatly appreciated!
There was a problem hiding this comment.
I’m currently facing a challenge with the scenario you mentioned: "If plugins A and B both replace the header and footer, and a site operator wants the header from A and the footer from B, this falls apart."
It is my understanding that this scenario is handled fully in #234
|
@regisb, @brian-smith-tcril, @hinakhadim, based on your feedback, I've tried another approach: Please review at your convenience. ;) |
|
Closing in favor of #240 |
It's now possible to configure frontend plugin slots across MFEs (notably, header and footer). This documents how this can be done as simply as possible.
See #199