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

[Miniflare 3] Fix source mapping of service workers #572

Merged
merged 1 commit into from
May 15, 2023

Conversation

mrbbot
Copy link
Contributor

@mrbbot mrbbot commented May 10, 2023

cloudflare/workerd#190 changed the script name for service workers to their service name. This change updates Miniflare's heuristics for locating source maps to work with this. It also adds some tests to ensure source mapping is working correctly for the different ways of specifying a script.

(to be merged after #566)

@mrbbot mrbbot added the tre Relating to Miniflare 3 label May 10, 2023
@mrbbot mrbbot requested a review from a team May 10, 2023 14:58
@changeset-bot
Copy link

changeset-bot bot commented May 10, 2023

⚠️ No Changeset found

Latest commit: 4837f96

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

Base automatically changed from tre-queues to tre May 12, 2023 16:24
@mrbbot mrbbot force-pushed the tre-service-worker-source-maps branch from 8fdb543 to 75f7b33 Compare May 12, 2023 16:35
Copy link
Contributor

@penalosa penalosa left a comment

Choose a reason for hiding this comment

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

All looks great! I'm not sure about the tests—I think it would be useful to check that the passed message is actually present in the error stack, since right now it doesn't look like that's being tested.

@mrbbot
Copy link
Contributor Author

mrbbot commented May 15, 2023

check that the passed message is actually present in the error stack

We do check it's in the error as the 2nd argument to throwsAsync(), but not specifically in the stack. Could be worth checking the stack too though... 🤔

cloudflare/workerd#190 changed the script name for service workers to
their service name. This change updates Miniflare's heuristics for
locating source maps to work with this. It also adds some tests to
ensure source mapping is working correctly for the different ways of
specifying a script.
@mrbbot mrbbot force-pushed the tre-service-worker-source-maps branch from 75f7b33 to 4837f96 Compare May 15, 2023 15:19
@mrbbot mrbbot merged commit e928666 into tre May 15, 2023
@mrbbot mrbbot deleted the tre-service-worker-source-maps branch May 15, 2023 16:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tre Relating to Miniflare 3
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants