Skip to content

SPIKE: Fix console warnings when loading UI#1386

Merged
abcampo-iry merged 17 commits intomainfrom
issues/1440
Mar 18, 2026
Merged

SPIKE: Fix console warnings when loading UI#1386
abcampo-iry merged 17 commits intomainfrom
issues/1440

Conversation

@abcampo-iry
Copy link
Contributor

@abcampo-iry abcampo-iry commented Mar 16, 2026

This is the result of a few hours iteration to remove noise from the console.
https://github.com/RaspberryPiFoundation/digital-editor-issues/issues/1140

Fixes applied:

  • Silent console.warn("PyodideWorker is not initialized") as I'm not sure if brings much value
  • defaultIndex not used an invalid prop
  • Missing key in DesignSystemButton
  • silent webpackOk
  • destruct in ScratchIntegrationHOC.jsx to avoid saying not valid props
  • icon warning not needed favicon since its in iframe
  • Two dedupers to avoid error/warning flooding, approached this way to be still throwing the warning but be easily removable:
    • Deduping errors coming from design system, needs to be addressed in source
    • Deduping errors coming from scratch they should be fixed in origin, or when updating
Screen.Recording.2026-03-16.at.19.28.24.mov

- The initial tab comes from: SkulptRunner.jsx:480
- That state is kept in sync with showVisualOutput in SkulptRunner.jsx:484
- No need for that prop
- avoid noise when this "DEPRECATED: icons as React elements will not be
  supported in future releases" is emitted several times
- In order to have locales we need to make defaultProjects async,
added helpers to de-ref async loading and wait for locales to be loaded
before shown
Copilot AI review requested due to automatic review settings March 16, 2026 18:28
@abcampo-iry abcampo-iry temporarily deployed to previews/1386/merge March 16, 2026 18:28 — with GitHub Actions Inactive
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR focuses on reducing console noise when loading the UI (web-component + scratch editor), primarily by silencing/avoiding known noisy warnings and adding deduplication for repeated warnings.

Changes:

  • Add warning dedupers for Scratch editor and Design System warnings, and invoke them at entrypoints.
  • Adjust default-project creation to be locale-aware without triggering invalid React prop warnings (async default project creation + updated hook/test).
  • Fix/avoid several React warnings (missing key, invalid defaultIndex prop, favicon warning, suppress webpackOk messages).

Reviewed changes

Copilot reviewed 13 out of 16 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
src/web-component.js Installs Design System warning deduper during web-component startup.
src/web-component.html Adds a data-URL favicon to prevent favicon-related console warnings.
src/utils/defaultProjects.js Introduces async factory functions for localized default projects and avoids early i18n evaluation.
src/utils/dedupeScratchWarnings.js New utility to dedupe common Scratch/React warnings (via console monkey-patching).
src/utils/dedupeDesignSystemWarnings.js New utility to dedupe a specific Design System icon deprecation warning.
src/utils/ResizableWithHandle.js Minor formatting/indent fix in handle component selection.
src/scratch.jsx Installs Scratch warning deduper at scratch entrypoint.
src/hooks/useProject.js Switches default project creation to async localized factory and avoids dispatch after unmount.
src/hooks/useProject.test.js Updates expectations to account for async default project creation.
src/containers/WebComponentLoader.jsx Works around React warning by copying ToastContainer defaults and removing defaultProps.
src/components/ScratchEditor/ScratchIntegrationHOC.jsx Suppresses webpackOk postMessage noise and prevents leaking invalid props to wrapped component.
src/components/Menus/Sidebar/Sidebar.jsx Formatting-only ternary indentation change.
src/components/Menus/Sidebar/FilePanel/FilePanel.jsx Adds missing key for a button rendered in an array.
src/components/Editor/Runners/PythonRunner/SkulptRunner/SkulptRunner.jsx Removes invalid defaultIndex prop usage on Tabs.
src/components/Editor/Runners/PythonRunner/PyodideRunner/PyodideRunner.jsx Returns null instead of warning+implicit undefined when worker isn’t initialized.
src/components/Editor/Runners/PythonRunner/OutputViewToggle.jsx Formatting-only ternary indentation change.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@abcampo-iry abcampo-iry temporarily deployed to previews/1386/merge March 16, 2026 18:32 — with GitHub Actions Inactive
@abcampo-iry abcampo-iry temporarily deployed to previews/1386/merge March 16, 2026 18:40 — with GitHub Actions Inactive
python: defaultPythonProject,
html: defaultHtmlProject,
};

Copy link
Contributor

Choose a reason for hiding this comment

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

The changes in this file are the only ones that aren't obviously clear to me -

Will this have any user facing behaviour change?
Is there any tests we could add for this file?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the most complicated change imho.

There is a race condition:

  • If we use i18n.t it causes issues because it's not initialized, solution:
    • Start with a given string, it could be empty but the string is safer.
    • Then find the right i18n value.
    • I did a lot of tests with this one, and i didn't want to spend more time, but i could try cleaning up, this was the version I made to work without not a very invasive change
    • I think an alternative would be to return the const async conditional to have initailized the i18n, maybe add an extra effect in the component.
    • But this felt like smallest change i could think of.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This also fix this state that starts by default:

image

@zetter-rpf
Copy link
Contributor

This is great progress on the warnings

  • Don't forget to link to the issue in the PR description - I know you have the branch name, but since we work across multiple projects, the issue number can be hard to find
  • Are there any remaining logs to the console that is worth looking at another time after this? If so, could you make another issue?

@abcampo-iry
Copy link
Contributor Author

abcampo-iry commented Mar 17, 2026

I think there are more warnings, but they are not as noisy, some of them are more like developing console logs?
I've cleaned up some of them after your comments

@abcampo-iry abcampo-iry temporarily deployed to previews/1386/merge March 17, 2026 10:35 — with GitHub Actions Inactive
Copy link
Contributor

@zetter-rpf zetter-rpf left a comment

Choose a reason for hiding this comment

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

thanks for fixing all the warnings!

@abcampo-iry abcampo-iry merged commit 584e8c9 into main Mar 18, 2026
7 checks passed
@abcampo-iry abcampo-iry deleted the issues/1440 branch March 18, 2026 10:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants