feat(legacy): inline yari#852
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
There was a problem hiding this comment.
CodeQL found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.
|
6cce294 was deployed to: https://fred-pr852.review.mdn.allizom.net/ |
…font Fred already declares @font-face for Inter in components/font/inter.css. BarlowCondensed-SemiBold.woff2 is only used by fred's scrim-inline (which ships its own copy).
17 SVGs had no className/import/url() reference anywhere; remove the files and the corresponding entries from the $icons map.
Plus updates renders one icon per BCD browser via browserToIconName().
Restore chrome, edge, opera, samsunginternet — covers chrome_android,
opera_android, samsunginternet_android via the split("_")[0] fallback.
No references in code or styles after the curriculum subtree was removed.
RenderSideBar (and its SidebarLeaf/SidebarLeaflets internals) had no caller; only SidebarContainer is imported (by static-page).
Plus's only consumer (StaticPage) never sets sidebarHTML, sidebarMacro, or related_content. Drop the inert SidebarFilter, data-macro attribute, and the dangerouslySetInnerHTML-driven scroll-to-current effect. Delete the now-orphaned filter.tsx, filter.scss, and SidebarFilterer.ts.
After removing the sidebar filter UI, handleSidebarClick only ever fires with macro=\"?\" and never with a filter value. Drop the hook from glean-context, delete sidebar-click.ts, and remove the four orphaned SIDEBAR_* constants.
Convert via synp, which preserves the resolved versions from yarn.lock exactly. libs/play already used npm.
Drop the yarn-specific install:all script and packageManager pin in vendor/yari, and switch fred's root prepare hook to \`npm ci\`. The vendor/yari prepare now chains \`npm ci\` across each nested package-lock.json.
Declare libs/* and client/pwa as workspaces of vendor/yari and drop the install:all/prepare scripts — \`npm ci\` now installs everything in one shot. Nested package-lock.json files collapse into a single root lockfile; node_modules hoist except where versions conflict (dexie pins 4.0.11 in client/pwa vs ^4.2.0 at the root).
Matches the root and lets npm hoist a single dexie copy.
The --allow-empty-input and --ignore-path .stylelintignore additions weren't needed: vendor/yari has no .css files (only .scss), so the **/*.css glob never picks anything up, and the per-file glob already filters staged files.
vendor/yari has no .css files, only .scss — stylelint's **/*.css glob never picks them up, so excluding vendor/ is unnecessary.
downshift had no callers in fred, components, hooks, or vendor/yari — it was the sole transitive source of React 19 in fred's tree. With it gone, vendor/yari's React 18 is the only React, so the legacy bundle's \`react\` resolve alias is no longer needed.
Drop the obsolete \`packageManager: yarn\` quirk; correct the PWA pre-cache description (path is /static/legacy/asset-manifest.json, shape is string[]); update the install/usage section to reflect the single-lockfile workspace setup.
These are the Glean schema source files; the committed generated/*.ts were produced from them by glean-parser. Even though we don't currently regenerate, keeping the yaml as the source of truth for what telemetry we collect.
| new rspack.HtmlRspackPlugin({ | ||
| inject: true, | ||
| chunks: ["yari"], | ||
| filename: "index.[contenthash].html", | ||
| template: "node_modules/@mdn/yari/client/public/index.html", | ||
| }), | ||
| new RspackManifestPlugin({ | ||
| fileName: "asset-manifest.json", | ||
| generate: (_seed, files) => | ||
| files | ||
| .map((file) => file.path) | ||
| .filter((path) => !path.endsWith(".map")), | ||
| }), |
There was a problem hiding this comment.
@LeoMcA Do you remember what these were for?
There was a problem hiding this comment.
These are needed for offline mode to work:
fred/vendor/yari/client/pwa/src/service-worker.ts
Lines 41 to 46 in 9c3a2f9
LeoMcA
left a comment
LeoMcA
left a comment
There was a problem hiding this comment.
Sorry, should've thought a bit more about this before beginning to review (or even encouraging you to do it!) - we need pretty much all of yari to render offline mode.
We could put everything back, but - even though it's a bit of a silly reason - I'm not sure I want to clutter up the git tree like that, especially with stuff we won't be wanting to edit.
Perhaps we should keep using the npm yari for now to render offline? That has the added advantage that we won't accidentally break something being used for that, and allows us to modify/slim/convert as much of the vendored code as we want.
Longer term it would be fun to get fred's server components running in-browser, and I have some ideas how to achieve that - so it doesn't have to be this way forever.
| "@zip.js/zip.js": "^2.8.26", | ||
| "css-loader": "^7.1.4", | ||
| "css-minimizer-webpack-plugin": "^8.0.0", | ||
| "dexie": "^4.4.2", |
| "postcss-preset-env": "^11.2.1", | ||
| "prettier": "^3.8.3", | ||
| "resolve-url-loader": "^5.0.0", | ||
| "rspack-manifest-plugin": "^5.2.1", |
There was a problem hiding this comment.
Let's add it back for offline mode
| "@wdio/local-runner": "^9.27.0", | ||
| "@wdio/mocha-framework": "^9.27.0", | ||
| "@wdio/spec-reporter": "^9.27.0", | ||
| "@zip.js/zip.js": "^2.8.26", |
| - **`client/src/`** — React/Lit source for the legacy bundle. Fred's [`legacy/index.tsx`](../../legacy/index.tsx) renders `<Plus />` from this tree. Subset kept: `plus/`, `ui/`, `document/`, `lit/`, `playground/`, `settings/`, `telemetry/`, `homepage/static-page/` (used by Plus docs), plus shared providers (`user-context.tsx`, `ui-context.tsx`, `placement-context.tsx`) and utilities. Yari's full SPA router and the homepage/blog/community/curriculum/observatory/etc. routes have all been removed. | ||
| - **`client/pwa/src/`** — service worker source, used by the rspack `service-worker` entry in [rspack.config.js](../../rspack.config.js). Note: the offline pre-cache install handler currently no-ops because it expects a manifest format and path that fred doesn't emit. Fixing that would require additional work (see "Known limitations" below). | ||
| - **`client/public/assets/`** — illustration/icon SVGs and PNGs referenced by Plus React components. | ||
| - **`libs/play/`** — playground runner used at runtime by fred's [server.js](../../server.js) (`handleRunner`). Not bundled by rspack. |
There was a problem hiding this comment.
Might be worth pointing out this was pre-existing (and we should come up with a better solution now it's in dex!)
| name: "legacy", | ||
| entry: { | ||
| index: "./legacy/index.tsx", | ||
| yari: "./node_modules/@mdn/yari/client/src/index.tsx", |
There was a problem hiding this comment.
Ah yeah, no: this is a problem - we need this file (which has been cleaned up from the vendored code) in order to render the offline JSONs we get from the differy zip.
There was a problem hiding this comment.
So if the differy zip would contain HTML files, would that solve our issues?
There was a problem hiding this comment.
Yes, though I reckon it would be quite large, and prone to frequent re-downloads (every dependency update would change some aspect of each page). We would perhaps also have to bundle the static assets in (HTML/CSS): we could cache them directly from the site itself, but they could diverge from the ones used in the differy build.
My gut says getting fred server components to run in browser would be easier (and quite interesting!)
Easiest would just be restoring this line and continuing to build the "offline yari" from npm.
There was a problem hiding this comment.
Easiest would just be restoring this line and continuing to build the "offline yari" from npm.
The benefit is very small if we still keep @mdn/yari as a dependency, unless it moves to devDependencies?
There was a problem hiding this comment.
yeah we could put it in dev or optional
There was a problem hiding this comment.
My gut says getting fred server components to run in browser would be easier (and quite interesting!)
Very unrefined, but here's my <1hr hacked together attempt at rendering fred server components in the browser - it works! #1546
4 participants
Description
Inlines all parts of yari required for MDN Plus.
Motivation
We want to archive the mdn/yari repository.
Additional details
Preview:
Related issues and pull requests
Part of #848.