Conversation
|
Cursor Agent can help with this pull request. Just |
81f8f2b to
622747f
Compare
622747f to
11f0fe9
Compare
11f0fe9 to
7f4e2b9
Compare
1b1f1c3 to
fe3bf52
Compare
There was a problem hiding this comment.
Pull request overview
This PR targets removal of yarn build deprecation warnings by modernizing the Sass toolchain/config and updating related dependency data, while also aligning Plotly styling/assets and adding a Plotly-focused Python sample project for validation.
Changes:
- Upgrade
sass-loaderand configure it to use the modern Sass JS API, including adding Sass load paths. - Update SCSS imports/asset references (design system, toastify, material symbols, plotly CSS) to avoid deprecated patterns.
- Refresh dependency lock data (
caniuse-lite) and bumpplotly.js; add a newpython-plotlysample project and link.
Reviewed changes
Copilot reviewed 7 out of 8 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
yarn.lock |
Updates resolved versions/checksums for sass-loader, plotly.js, and caniuse-lite to eliminate build warnings. |
webpack.config.js |
Switches sass-loader to modern API mode and sets Sass loadPaths for module resolution. |
src/web-component.html |
Adds python-plotly to the sample project picker. |
src/projects/python-plotly.json |
Introduces a new Python sample that exercises Plotly support. |
src/assets/stylesheets/InternalStyles.scss |
Removes Toastify SCSS import (Toastify styling now sourced via external stylesheet bundle). |
src/assets/stylesheets/ExternalStyles.scss |
Updates/standardizes external library style imports, including Plotly/Toastify/material symbols. |
package.json |
Bumps sass-loader and plotly.js dependency versions to match warning-fix strategy. |
AGENTS.md |
Updates contributor/agent guidance (notably around changelog/release notes). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
fe3bf52 to
1b1f1c3
Compare
Warning fixed: - "Sass @import rules are deprecated" emitted from "react-toastify/scss/main.scss" during yarn build. Approach: - Stop compiling Toastify Sass source and import the packages compiled CSS (react-toastify/dist/ReactToastify.css) instead. - Remove the upstream SCSS @use from InternalStyles.scss. Why this approach: - Preserves Toastify styling behavior while eliminating upstream Sass deprecation output in our build. - Avoids carrying or maintaining local forks of third-party styles. Alternatives considered: - Keep importing SCSS and suppress warnings globally: rejected because it hides actionable deprecations. - Patch Toastify Sass locally: rejected due to maintenance overhead and divergence from upstream. Co-authored-by: Chris Zetter <zetter-rpf@users.noreply.github.com>
Warnings fixed: - "Sass @import rules are deprecated" from material-symbols/sharp.scss. - "Global built-in functions are deprecated" (to-lower-case, str-index, str-slice, str-length) from material-symbols/_core.scss. Approach: - Replace material-symbols/sharp.scss import with material-symbols/sharp.css in ExternalStyles.scss. Why this approach: - Uses upstream distributed CSS, so our build no longer compiles third-party Sass that triggers deprecation warnings. - Keeps the same font-face and utility class output with minimal risk. Alternatives considered: - Keep SCSS and silence deprecations: rejected because it masks dependency issues broadly. - Patch material-symbols Sass locally: rejected due to long-term maintenance burden. Co-authored-by: Chris Zetter <zetter-rpf@users.noreply.github.com>
Warning fixed: - "The legacy JS API is deprecated and will be removed in Dart Sass 2.0.0." repeated during yarn build. Approach: - Upgrade sass-loader from v10 to v13 (Node 16 compatible) and configure sass-loader with api: "modern". - Add Sass load paths for node_modules so package imports resolve under the modern API. - Update design-system-react @forward path to its explicit distributed SCSS entrypoint for reliable modern resolution. Why this approach: - Removes dependence on the deprecated Sass legacy API instead of suppressing the warning. - Keeps compatibility with current CI Node runtime while modernizing Sass integration. Alternatives considered: - Stay on sass-loader v10 and suppress the legacy API warning: rejected because this leaves deprecated behavior in place. - Jump to sass-loader v14+: rejected because those versions require newer Node than current CI. Co-authored-by: Chris Zetter <zetter-rpf@users.noreply.github.com>
Warning fixed: - "Sass @import rules are deprecated" emitted from plotly.js/src/css/style.scss imports during yarn build. Use the built plotly styles Newer versions of plotly have the built styles that we can use.
1b1f1c3 to
6aa4021
Compare
Warning fixed: - "Browserslist: caniuse-lite is outdated" shown during yarn build. Approach: - Refresh caniuse-lite/baseline browser mapping data in the lockfile using update-browserslist-db. Why this approach: - This directly resolves the warning with the officially recommended update path. - Keeps target browser data current for autoprefixing/transpilation decisions. Alternatives considered: - Ignore the warning: rejected because stale browser data can skew build targets. - Pin an older browserslist stack: rejected because it delays routine ecosystem updates. Co-authored-by: Chris Zetter <zetter-rpf@users.noreply.github.com>
Changes: - Clarify that CHANGELOG.md should not be updated; GitHub Releases are the source of truth. - Add commit guidance requiring descriptive messages with rationale and alternatives when relevant. Why: - Aligns repository instructions with current release-note process. - Improves commit history quality and review context for future maintenance. Co-authored-by: Chris Zetter <zetter-rpf@users.noreply.github.com>
Plotly does have an e2e test, but I wanted to be able to check the styling of it after the 3.3 upgrade
6aa4021 to
5cdb6a4
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 8 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
adrian-rpf
left a comment
There was a problem hiding this comment.
Looks like wise changes.
Did wonder about putting Ploty area as a separate issue but overall it makes sense so ignore my early comments.
There was a problem hiding this comment.
is this linked to the deprecation warnings?
There was a problem hiding this comment.
Yeah I guess you were looking at testing it. Makes more sense.
Closes https://github.com/RaspberryPiFoundation/digital-editor-issues/issues/1139
Remove all
yarn builddeprecation warnings by updating Sass imports,sass-loaderconfiguration, andcaniuse-litedata.See commits for more details.