feat: expose PARAGON as a global variable
#365
Conversation
adamstankiewicz
force-pushed
the
ags/2321
branch
2 times, most recently
from
9c05f9e to
0aff13d
Compare
adamstankiewicz
changed the title
PARAGON_VERSION as a global variablePARAGON as a global variable
adamstankiewicz
force-pushed
the
ags/2321
branch
2 times, most recently
from
7ec92fc to
56a285e
Compare
| @@ -19,4 +30,16 @@ module.exports = { | |||
| }, | |||
| extensions: ['.js', '.jsx'], | |||
| }, | |||
| optimization: { | |||
There was a problem hiding this comment.
I see the various chunks: ['app'] below, and this optimization block here too. What's the net effect of how we've changed the chunking? What do the cacheGroups do for us?
There was a problem hiding this comment.
Put another way, it may be worth a comment in the config here describing what we're doing to the optimization and how this differs from the default (I think you're leaving the chunking the same and just adding this caching thing, but that's only cause I've stared at a lot of these in my day 😉)
There was a problem hiding this comment.
Correct, the chunking is staying the same as is it today, with the addition of splitting out the "locally" installed already-compiled CSS files from @edx/paragon and @edx/brand. I will add a comment :)
That said, we probably should find some ways to optimize our MFE Webpack bundles (e.g., test hypothesis that many small JS files for the node_modules is better than 1 larger JS file, recommend/implement a strategy for code splitting for MFEs): openedx/wg-frontend#173
This PR purpose is to test/demo parago design tokens simliar to this one for the profile openedx/frontend-app-profile/pull/764 it override the following depns as seen in package.json - paragon alpha - openedx/frontend-build/pull/365 - openedx/frontend-platform/pull/440 - openedx/frontend-component-header/pull/351 - openedx/frontend-component-footer/pull/303 Conclousion so far: - There is an extra library that learning depends on which needs to support paragon; `frontend-lib-learning-assistant` and also `frontend-lib-special-exams` - It would be great to have a gudie or a document to look at, while doing the conversion that would **map variable from who it was used/named before to the name in design tokens** - I was stuck in the end with compliation error, that wepack couldn't find `Modal` exported from paragon.
|
Hey @adamstankiewicz, What is the current status of this PR, is it ready to review and merge? |
| if (fs.existsSync(envConfigFile)) { | ||
| envConfig = require(envConfigFile); | ||
| } | ||
| // const paragonThemeUrls = mfeRuntimeApiConfig?.PARAGON_THEME_URLS |
There was a problem hiding this comment.
Hello, I would suggest removing these comments if they are not intended to be used in the future.
| const response = await axios.get(url); | ||
| return response.data; |
There was a problem hiding this comment.
Could we consider incorporating a try/catch block for this operation? It would be beneficial to anticipate and handle potential failures in the request.
| // Add process env vars. Currently used only for setting the | ||
| // server port and the publicPath | ||
| dotenv.config({ | ||
| path: path.resolve(process.cwd(), '.env.development'), |
There was a problem hiding this comment.
This makes that the build(npm run build) process uses env.development setttings instead of the env
Co-authored-by: Diana Olarte <[email protected]>
|
I'd like to put this (and openedx/frontend-platform#440) back on our radar, now that Redwood is cut and OEP-65 has been merged. @davidjoy, interested in your take in particular: do you feel this (external CSS stylesheets) is something we should push before, in parallel, as part of, or after OEP-65? |
|
@adamstankiewicz may have a more comprehensive view of this than I do, as I'm trying to page this paragon work back in to my brain. But my sense of it (including openedx/frontend-platform#440) is that:
|
@davidjoy When you say "nothing special will be necessary" for sharing a stylesheet between MFEs as it will become the default behavior once we have a "shell" app, are you referring to the changes in the openedx/frontend-platform#440 no longer being necessary? That is, the frontend-platform PR enables support for using the Paragon CSS from a CDN URL instead of importing directly from the
Correct, there is no official dark mode (yet). The design tokens work in Paragon sets up the foundational ground work to support a dark mode in the future (and maybe a high-contrast mode even longer term) and the frontend-build/platform PRs try to support those future use cases. The actual theme switching capabilities is not a strict requirement for the design tokens initial release, though, but would like to ensure the technical design for consuming the updated Paragon CSS from design tokens can be readily extended to support themes in the future.
@arbrandes @davidjoy Supporting the external CSS stylesheets (i.e., loading Paragon's CSS via CDN url vs. importing from Likely the biggest overlap with OEP-65 and Paragon design tokens is not so much whether we adopt the external CSS stylesheets proposed by these 2 PRs, but more so Paragon migrating from SCSS -> CSS variables. That is, specifically, around the current styles challenge where custom SCSS files in MFEs using Paragon's SCSS variables need to either be imported by the MFE's styles entry point (e.g., This current challenge around needing to import Paragon SCSS variables before using the variables in custom SCSS files will be a non-issue after the SCSS -> CSS migration as part of Paragon design tokens. That is, custom SCSS will be able to reference Paragon's CSS variables without needing to import anything from Paragon into the custom SCSS file(s) first. When a "guest" MFE has custom styles relying on Paragon's variables, but Paragon's CSS is provided by the "shell", we would want to be sure we do not risk introducing duplicate Paragon styles, as some MFEs are doing today. As it relates to OEP-65 and its shared CSS in the "shell", we could probably start by importing Paragon's CSS from Hope this helps! |
|
Thanks Adam!
I'm just saying that if the shell loads the brand and paragon CSS, then all the other MFEs loaded into it don't need to... assuming they want a compatible version. This was under the assumption that the external CDN URL was only about downloading the stylesheet once; sounds like there's more to it than that. |
|
Closed in favor of #546, which picked up where this PR left off. |
openedx/paragon#2321
In this post, @xitij2000 had a great suggestion to support version substitution on the Paragon theme urls such that if the theme url specified in configuration looks something like,
https://cdn.jsdelivr.net/npm/@edx/paragon@$paragonVersion/dist/core.css, we could replace the$paragonVersionwith the version of the locally installed Paragon in the application itself.This approach should help mitigate the concern around the coupling between the Paragon version installed in
package.jsonin consuming MFEs (related to this PR) as, if the theme URLs contain$paragonVersion, consumers won't need to worry about updating Paragon versions in two places, or using an incompatible version of the Paragon theme between the version specified in configuration and the locally installed Paragon version.Output
If there is no compatible version of
@edx/paragoninstalled (i.e., does not include aparagon-theme.jsonfile in itsdistdirectory innode_modules), falls back to create anundefinedglobal variable, which prevents consuming MFE runtimes from breaking due to usage of an undefinedPARAGONglobal variable:Consuming libraries (e.g.,
@edx/frontend-platform) should not expectPARAGON_THEMEto always be defined.Development build (
npm start)Production build (
npm run build)Note: includes the
[contentHash]due to options inwebpack.config.prod.js. When the Paragon version changes, we will invalidate the cached fileparagon-theme-{url}.{hash}.jsdue to the{hash}changing when the files themselves change.