search

openedx / frontend-app-learning

Front-end for the Open edX course experience, implemented using React and Paragon.
GNU Affero General Public License v3.0
46 stars 207 forks source link

Don't include Learning Assistant in the bundle #1482

Open bradenmacdonald opened 2 months ago

bradenmacdonald commented 2 months ago

To reduce the bundle size for everyone, we need to make a few fixes to https://github.com/edx/frontend-lib-learning-assistant :

  1. It's peerDependencies are overly specified. For example, it shouldn't pin redux to the exact version 4.1.2 as that blocks us from bumping the version in frontend-app-learning. Specifying ^4 or ^4.1 is fine.
  2. It should not be part of frontend-app-learning's redux store. Instead it should have its own store and its own <Provider>. Or better yet, don't use redux at all.
  3. The <Xpert> component inside <Chat> should use React.lazy so that the learning-assistant code is only loaded on demand, and not always bundled into the main learning MFE app bundle.
  4. Optional: It should be a plugin using frontend-plugin-framework, and not even mentioned in the code at all.

Numbers 2 and 3 are the main thing I'd like to achieve here, as having both of those will result in a bundle size reduction.

bradenmacdonald commented 2 months ago

FYI @alangsto @MichaelRoytman - not quite sure who to ping about this. Could you let me know if you have any existing plans along these lines?

alangsto commented 2 months ago

@bradenmacdonald I will bring this up to our team on Monday and get back to you! Thanks for pinging us.

alangsto commented 2 months ago

Hi @bradenmacdonald, thank you again for bringing this to our attention! Two clarifying questions:

Thanks!

bradenmacdonald commented 2 months ago

@alangsto Bundle size is one of the main factors affecting performance, and especially how quickly the Learning MFE will load for users on their first visit, and on subsequent visits after a new version has been deployed. Basically at the moment, users who are visiting any site other than edx.org are still downloading the frontend code for Xpert / Learning Assistant even though that's an edx-specific feature. So by moving it to be "lazy-loaded", those users will see the MFE loading faster because they no longer need to wait to download that code before it's initialized.

In this case, I would not say it's very high priority as the code size is not that big. But we want to get in the habit of keeping the bundle size small and using code-splitting (lazy loading) since those are best practices. That's why I'm adding BundleWatch to the repo as recommended by OEP-67, and tools like BundlePhobia and webpack-bundle-analyzer exist (the latter is already built in to our build).

What is high priority for me though is not particularly related, but the fact that the peerDependencies of frontend-lib-learning-assistant are blocking some dependency version bumps, so I'd love to see those updated sooner. I can open a PR to do that though.

alangsto commented 2 months ago

@bradenmacdonald thank you for the added context, this is very helpful! I'm going to create a discovery ticket for my team to look into the options you listed to determine the scope. Once we have completed the discovery I will share that doc with you.

bradenmacdonald commented 2 months ago

@alangsto Great, thanks! I should also mention, it will improve performance for everyone on edx.org as well, if they're not in a course/context that's using learning assistant.