Skip to content
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

fix: ensure webpack.dev.config.js includes quietDeps: true, applies injected <link> styles once loaded, example app updates to show ParagonWebpackPlugin output #584

Merged
merged 1 commit into from
Aug 30, 2024

Conversation

adamstankiewicz
Copy link
Member

@adamstankiewicz adamstankiewicz commented Aug 30, 2024

CHANGELOG

Fix quietDeps: true regression during local dev

As part of #546, some of the styles-related Webpack config in webpack.dev.config.js were refactored to a helper function getStyleUseConfig. As part of this refactor, the quietDeps option passed in sassOptions was lost.

Without quietDeps: true, Webpack will throw a (loud) warning in the browser/console given some usage of deprecated Sass syntax in some/all MFEs when running npm start to run the app.

image

This PR ensures quietDeps: true is added back to sassOptions to remove deprecation errors surfaced by Webpack.

Applies injected <link> styles onload

Currently, frontend-build injects <link> elements with rel="preload" as="style", but does not actually apply the styles itself.

This PR adds an onload handler on the injected <link> elements to ensure the stylesheets are applied as early as possible, e.g. without relying on frontend-platform's handling of PARAGON_THEME_URLS.

In short, frontend-build is primarily be responsible for applying Paragon/brand styles from configuration on initial page load whereas frontend-platform is responsible to handling any runtime configuration changes of the configuration (e.g., as this could be used with the MFE runtime configuration API, or if env.config.js is dynamic).

Example MFE app

  • Imports Paragon SCSS (v22) into example app to give it styles 🎨 (i.e., @use '@openedx/paragon/scss/core/core';)
    • Note: this should change once Paragon v23 is released.
  • Introduces ParagonPreview in the example app to more easily QA changes in ParagonWebpackPlugin, carried over from my original PR.

With Paragon v22

Frame 1

With Paragon / brand package (@edx/elm-theme, github) v23 (alpha)

Frame 1

@@ -15,8 +15,9 @@
"author": "",
"license": "ISC",
"dependencies": {
"react": "^16.14.0",
"react-dom": "^16.14.0"
"@openedx/paragon": "22.7.0",
Copy link
Member Author

@adamstankiewicz adamstankiewicz Aug 30, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[inform] Now installs @openedx/paragon in example app, which now includes a ParagonPreview component to render the output of the generated PARAGON_THEME global variable. It also includes a basic component preview of Button, which will be useful when using the example app for any of brand package overrides with the ParagonWebpackPlugin (e.g., to sure it's applying correctly).

@@ -38,6 +39,7 @@ export default function App() {
<p>env.config.js integer test: {Number.isInteger(config.INTEGER_VALUE) ? 'It was an integer. Great.' : 'It was not an integer! Why not? '}</p>
<h2>Right-to-left language handling tests</h2>
<p className="text-align-right">I&apos;m aligned right, but left in RTL.</p>
<ParagonPreview />
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[inform] This is carried over from my original PR for ParagonWebpackPlugin.

@@ -74,6 +74,7 @@ function insertStylesheetsIntoDocument({
rel="preload"
as="style"
href="${url}"
onload="this.rel='stylesheet';"
Copy link
Member Author

@adamstankiewicz adamstankiewicz Aug 30, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[inform] Ensures the pre-loaded stylesheet is applied once its successfully loaded. E.g., it no longer relies on the corresponding frontend-platform PR to apply the injected <link> styles, ensuring they're applied as early as possible in the initial page load.

Edit: frontend-platform is more responsible for handling when:

  • there's an initial attempt at loading CDN urls fails from frontend-build, frontend-platform will retry, and then fallback to the locally installed version.
  • the PARAGON_THEME_URLS config changes at runtime (e.g., via MFE runtime config API or a dynamic value with env.config).

@adamstankiewicz adamstankiewicz marked this pull request as ready for review August 30, 2024 18:14
Copy link
Contributor

@brian-smith-tcril brian-smith-tcril left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Since this is all in 1 commit would you mind updating the commit message to mention all the changes (or just changing the PR title and we can squash)?

@adamstankiewicz adamstankiewicz changed the title fix: ensure webpack.dev.config.js includes quietDeps: true fix: ensure webpack.dev.config.js includes quietDeps: true, applies injected <link> styles once loaded, example app updates to show ParagonWebpackPlugin output Aug 30, 2024
@adamstankiewicz adamstankiewicz merged commit 4c7abe1 into master Aug 30, 2024
5 checks passed
@adamstankiewicz adamstankiewicz deleted the ags/fix-dev-quiet-deps branch August 30, 2024 19:06
@openedx-semantic-release-bot

🎉 This PR is included in version 14.1.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

@openedx-semantic-release-bot

🎉 This PR is included in version 15.0.0-alpha.16 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants