overhangio / tutor-mfe

This plugin makes it possible to easily add micro frontend (MFE) applications on top of an Open edX platform that runs with Tutor.
GNU Affero General Public License v3.0
22 stars 95 forks source link

Support Javascript configuration #199

Open arbrandes opened 7 months ago

arbrandes commented 7 months ago

Description

tutor-mfe doesn't currently support Javascript configuration, except potentially by the old, abandoned expedient of baking it into the image. The latter is obviously not something we want to go back to, so a way must be devised to take advantage of JS configuration dynamically.

Why should we support it?

Because plugins will require it.

Solution Space

A list of potential solutions, for discussion.

1. Extend mfe_config

Extend the mfe_config API so that JS configuration can also be served. Extend runtimeConfig() so that it makes use of it.

2. Refactor mfe_config

This is mostly the same as 1., except instead of piggy-backing on site_configuration, we write a dedicated admin page for it.

3. Move mfe_config to tutor-mfe

Note: this last option is the most similar to how Piral does - or can do - configuration. I suggest we explore it seriously, as it might make our jobs easier later.

Instead of relying on an edx-platform API, we build a mechanism in tutor-mfe to serve the correct JS file dynamically. We'd likely be able to leverage the fact that .env.config.js is just Javascript to make a request to a predictable location that Caddy can then service. This file would be user-overridable, so if the operator wants to do something fancy like fetching a different configuration depending on the domain, they totally could.

Pros:

Cons:

arbrandes commented 7 months ago

I'm looking for thoughts, here. Particularly from @felipemontoya and @regisb.

felipemontoya commented 7 months ago

In order to comment with my opinion here I had to go open the pandora box of frontend-plugin-framework. I was positively surprise by that project and I think there is a lot of value in incorporating that technology into the mfe stack for the project. I'm still missing one feature there, but that is irrelevant for this discussion.

Thinking about how to bring that capability here, I agree with you that putting js files in the site_configuration object is probably not the best way to go. I would also say the same thing about creating a new model with a dedicated admin page for storing js.

In the time I have been investigating the frontend-plugin-framework I've noticed how finicky the configuration file is. My take on this is that we should let code be code. Most likely with a repository and all, but specially giving whoever is writing the proper tools. E.g, syntax highlight and linters.

Now, about the option 3. Move mfe_config to tutor-mfe I don't think the mfe_config should be moved. I think we should add support for js config in a way that benefits a large groups of users of tutor-mfe. We can have examples and also a default js file with the out-of-the-box configuration for the project. Then we need to write in the necessary hooks (or any other replace/extend mechanisms) so that operators can provide the most advanced cases for themselves in the cases where editing the provided file is not enough. Then we deprecate env variables and mfe_config at the same time. This I suppose will take more than one release cycle to complete.

bradenmacdonald commented 7 months ago

The docs say that the config file can be either .js or .jsx. I also see the examples contain a lot of imports.

If that's the case, there is an additional layer of complexity which is that something (either the Django backend or the MFE container / webpack etc.) needs to convert the JSX to plain JS. (I would prefer to require either [A] that the config file is .js only, and can only import ESM modules; or [B] that the config file can be .ts/.tsx/.jsx/.js and have a simple, standard way of compiling it - either when the MFE container launches, or when the config is changed on the backend.) I don't think that we should be doing any JSX compilation or eval() of JS code at runtime.

regisb commented 7 months ago

The initial premise of microfrontends was:

  1. build once,
  2. deploy anywhere.

Then we realized it was a terrible idea, because we needed dynamic configuration. So that became:

  1. build once,
  2. deploy anywhere,
  3. with configuration served dynamically.

And now we are saying that configuration is more complex than we thought, so we need to build it. Thus microfrontends become:

  1. build once,
  2. deploy anywhere,
  3. then build configuration,
  4. and serve it dynamically.

I think that this additional complexity is unnecessary. We are going to transform configuration into code because we are making the incorrect assumption that plugin loading needs to reside in configuration. This does not have to be the case. Plugins should be loaded at build time. Then setting values no longer need to include js code.

Microfrontends are already a bloody mess. Can we try to make them simpler, and not more complex?

arbrandes commented 7 months ago

Plugins should be loaded at build time

I disagree. Even though this is not true now, plugins, like MFEs, should be loadable at runtime. It is the direction we're going with the Piral/module federation work. And as such, there's no way around having some form of dynamic configuration.

the incorrect assumption that plugin loading needs to reside in configuration

It is not an assumption, made without consideration. It was a conscious decision based on how Node, Webpack, and React work, and also what we envisioned frontend plugins to be: "dumb" collections of components, that the user then decides where to load via configuration.

Why? Because it's easy to imagine a user wanting to match the same plugins to the same UI slots, but in different ways. One may want to change the order the widgets appear. Another may want one widget to load on the left sidebar, as opposed to the right.

Microfrontends are already a bloody mess. Can we try to make them simpler, and not more complex?

They are indeed! :) And we're working on simplifying some things (via Piral/module federation), but adding a plugin system is not where you'd usually want to reduce complexity. I'd argue that it's actually impossible to do so without making at least some things less straightforward.

felipemontoya commented 7 months ago

I have also felt strongly that microfrontends are blody mess, however I disagree in that the goal is not simplicity but actual usability.

The process of:

1. build once,
2. deploy anywhere.

Didn't work because the built microfrontends where not usable. We needed to extract configuration from the build (not unlike the python edx-platform images) so that you did not need to fork away every time. This is what led to

1. build once,
2. deploy anywhere,
3. with configuration served dynamically.

but this didn't work because the configuration that could be served dynamically was not good enough to make them very usable. So for most instances we were back to having to fork the MFE and build for every single deploy instead of deploy anywhere.

So at this point we are pursuing a better configuration, that optionally requires a build, so that actually the "build once" promise is fullfilled.

Making a reactive, fast, user friendly interface is hard and the frontend landscape is hard and frustrating at times. As is making a scalable, flexible and fast backend. The move to microfrontends was championed by a team that had a high ratio of developers per site and we have not yet reached the same level of usability for sites that have way less than that, but we are working towards it.

arbrandes commented 7 months ago

Tangential note on MFEs in general: we have a brand new version of OEP-0065 that is intended to solve some of the frustrations we all know.

https://github.com/openedx/open-edx-proposals/pull/575

It's the Piral OEP, but without the Piral (because it added too much unneeded complexity. ;)

regisb commented 7 months ago

@arbrandes you are saying that we should have plugins loaded at runtime. @felipemontoya you are saying that configuration (optionally) requires a build.

Both of these propositions can't be achieved at the same time, right?

arbrandes commented 7 months ago

The configuration build step is currently only necessary to support jsx (and possibly tsx), as per @bradenmacdonald's comment above. We could go one of two ways, here: either only allow plain js for tutor-mfe (we don't care how the user creates it, manually or building them from jsx themselves), or incorporate an automatic build step whenever the file is changed. In the latter case, whether Tutor does the build itself or not will depend on how much we want Tutor to manage this configuration file.

Anyway, at first glance, env.config.js needs to be present at build time. Look how it is imported by frontend-platform. However, the way envConfig is loaded at runtime gives us the ability to make it dynamic:

 if (typeof envConfig === 'function') {
    config = await envConfig();
  }

We could have Tutor supply a static env.config.js at build time, populated with an envConfig() function that in turn fetches (prebuilt, importable) JS configuration dynamically from a stable (but itself configurable) location at runtime. The default could be Caddy serving the module, but the user could prefer to build their own JS config pipeline for advanced scenarios - so maybe make the URL patchable. Or let the user replace the whole env.config.js with their own, if they want.

As @felipemontoya put it, all this doesn't need to replace mfe_config - at least not yet. But it will be necessary if you actually want to use any plugins.

arbrandes commented 7 months ago

Of course, there's also the option of making Tutor aware of the JS config file syntax so it could be templated with patches, like everything else. Thus potentially hiding away some of the complexity from the user.

arbrandes commented 7 months ago

One further detail I neglected to bring up is that as things stand now, pre OEP-65, the plugin packages do need to be npm-installed at build time. In Tutor, this would be achieved via the post-npm-install hooks.

So, in practice, while we'd have a dynamic way to configure plugins, we'd still have to install them up front.

davidjoy commented 7 months ago

Small note: the reason that the env.config can be js or jsx is because while it is present at build time and outside the src directory, it's bundled by webpack as part of the source code and used at runtime. It's just like any other source file in that regard. For instance, adding build-time configuration to env.config.js (like a PORT for webpack-dev-server, for instance) is non-sensical, as the build doesn't actually get configured by the file, unlike .env which contains both build-time and runtime config (which is confusing).

Maybe I'm missing some internals of tutor, but so long as the file is something our webpack build can handle, we don't need to precompile it in any way; the build itself does that.

arbrandes commented 7 months ago

@davidjoy

as the file is something our webpack build can handle, we don't need to precompile it in any way; the build itself does that.

Maybe what you're missing is that currently Tutor relies on mfe_config for all env config, so that users can use a Tutor-provided pre-built Docker image while still being able to make some changes specific to their deployment (LMS and CMS base URLs, for instance). If JS config is to replace that, we'll need Tutor to be able to load it at runtime too.

regisb commented 7 months ago

I bet you a round of piña coladas for all commenters on this PR against a case of peanut butter (here is our family's favorite brand) that we will want to include jsx/tsx config files before Teak is out. Probably because plugins will want to import nodejs modules. Then we will realize that the right way to achieve that is to serve config values at runtime (in js), and to load plugins at build time (in jsx).

When that happens, I'll have a good reason to love microfrontends and I'll be grateful for them every time I spread chocolate chips on my peanut butter sandwiches.

felipemontoya commented 7 months ago

I think you are just trying to get a lifetime supply of peanut butter from Adolfo 😂. In my opinion we would want to include jsx/tsx config today if that had been proven to work for instances in the community. We will also want to include the css file with all the variables after paragon supports design-tokens. And should the Vite thing become a reality and need support here (which so far it looks like it doesn't) we will also come to this plugin to discuss it.

I have been circulating this idea in my mind, would it make sense to have 2 different plugins for MFE support in tutor? One where long term maintainability and build once, deploy everywhere is the driving goal and a second that has a lot less expectation of long term support but allows Frontend devs to have advanced building tools and extensions? If this is heretic dont burn me at the stake just yet, I am coming at this from the angle that maybe we cant find a compromise between lightweight easy to maintain instances and heavy modified frontends in a single plugin.

bradenmacdonald commented 7 months ago

serve config values at runtime (in js), and to load plugins at build time (in jsx).

That sounds right to me. All the possible/installed plugins should be baked into the MFE image at build time, and you can then enable/configure them dynamically using the MFE config API, which doesn't need to support JSX/TSX. The MFE can use ESM dynamic imports (await import('path/to/plugin.js')) to load each plugin, which is great because it forces proper code splitting of the bundle. Webpack, Vite, etc. already fully support this sort of thing, including auto-detecting what plugins may be imported and premptively bundling (as optional separate files) them during the build.

One where long term maintainability and build once, deploy everywhere is the driving goal and a second that has a lot less expectation of long term support but allows Frontend devs to have advanced building tools and extensions?

I'm actually kind of thinking along those lines too. I think Tutor MFE doesn't need to support "dev mode" for MFEs anymore, because I discovered that the way Tutor does MFE config makes it super easy to just run the MFE on your host. For example, with this change I made it so you can just run npm run start-tutor on an MFE on the host, and it auto-configures itself and integrates with the tutor devstack, but with higher performance (at least for Mac users; for linux users maybe not so big a difference).

hinakhadim commented 6 months ago

Just went through the above discussion, why are not we simply include plugin packages in post-npm-install and add those plugins in env.config.jsx. Then, copy env.config.jsx file from the tutor-mfe to docker container (similar as we do webpack.dev-tutor.config.js).

The only problem seems is that there are 10 MFEs (suppose) and I want to change different slots in all MFEs. I have to include all the plugins in one file env.config.jsx, causing overhead. As Authn MFE doesn't want to use plugins of other MFEs mentioned in env.config.jsx.

That's all my understanding from testing Footer (using Plugin approach) in Learner-dashboard MFE. Am I missing any crucial point?

arbrandes commented 6 months ago

why are not we simply include plugin packages in post-npm-install and add those plugins in env.config.jsx

@hinakhadim, for Redwood, that is likely all we're going to do. I'll whip up a PR that changes the README to document how to do it.

However, once plugins themselves can be loaded dynamically (as a consequence of OEP-0065), we'll also want to configure them dynamically. And in doing so, we could also deprecate the mfe_config API - and make it much more flexible - by building a Tutor-specific config system based on env.config.js. The latter is mostly what this issue is about.