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

Avoid embedding web-worker polyfill #12400

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

moio
Copy link
Contributor

@moio moio commented Oct 29, 2024

Summary

Fixes: #12511

TL;DR: Drop the web-worker polyfill. It's for ancient browsers and confuses tooling that works with performance captures.

QA NOTE: No user visible change expected, this is only to aid developers interpreting performance captures, eg. following https://github.com/rancherlabs/rancher-process/blob/main/docs/documentation/howtos/browser_performance_capture.md

Technical notes summary

The BLOB code from web-worker is minified, and contains source maps that are then passed over by webpack - that creates a situation of minified code inside BLOB inside minified code which confuses some tools, eg.

jantimon/chrome-profile-sourcemap-resolver#4

That could be fixed, but the only reason to do so would be supporting very old (pre-2013) browsers, so we are better off removing the polyfill altogether.

Areas or cases that should be tested

Nothing in particular, exisiting tests should abundantly cover this.

Areas which could experience regressions

None expected.

Checklist

  • The PR is linked to an issue and the linked issue has a Milestone, or no issue is needed
  • The PR has a Milestone
  • The PR template has been filled out
  • The PR has been self reviewed
  • The PR has a reviewer assigned
  • The PR has automated tests or clear instructions for manual tests and the linked issue has appropriate QA labels, or tests are not needed
  • The PR has reviewed with UX and tested in light and dark mode, or there are no UX changes

The BLOB code from web-worker is minified, and contains source maps that are then
passed over by webpack - that creates a situation of minified code inside BLOB
inside minified code which confuses some tools, eg.

jantimon/chrome-profile-sourcemap-resolver#4

That could be fixed, but the only reason to do so would be supporting very old
(pre-2013) browsers, so we are better off removing the polyfill altogether.

Signed-off-by: Silvio Moioli <silvio@moioli.net>
@moio moio added this to the v2.10.1 milestone Oct 29, 2024
@moio moio requested a review from richard-cox October 29, 2024 10:38
@moio moio assigned moio and unassigned richard-cox Oct 29, 2024
@richard-cox richard-cox modified the milestones: v2.10.1, v2.11.0 Oct 29, 2024
@moio
Copy link
Contributor Author

moio commented Oct 29, 2024

End-to-end failures seem unrelated to me, but please advise if they are not.

@nwmac nwmac modified the milestones: v2.12.0, v2.11.0 Nov 1, 2024
@richard-cox
Copy link
Member

Fallback came in via #6563

Copy link
Member

@richard-cox richard-cox left a comment

Choose a reason for hiding this comment

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

Think this is ok, but would like to confirm a prod build running remotely works (given #6563). Just making one now (richard-dev).

If that's fine then we just need to wait for 2.10 branching (happening right now)

@richard-cox
Copy link
Member

@moio same for this one

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.

Embedding web-worker polyfill breaks sourcemap tools
3 participants