Skip to content
This repository has been archived by the owner on Oct 1, 2024. It is now read-only.

fix:loader-module-webpack #2816

Merged
merged 4 commits into from
Sep 24, 2024

Conversation

admirsaheta
Copy link
Contributor

Description

Fixes (issue #2807)

This PR addresses an issue with the webpack loader in @shopify/web-worker where output.chunkFilename could be either a string or a function returning a string.
The loader previously assumed it was always a string, leading to TypeError: file.includes is not a function errors.
The solution now checks if output.chunkFilename is a function and handles it correctly.

Changes Made

  • Default Values: Default values [worker].js are provided for compiler.options.output.filename and compiler.options.output.chunkFilename in case they are undefined.
  • Type Handling in addWorkerSubExtension:
  • If file is a string, the function directly performs the replacement to add the .worker suffix.
  • If file is a function, the function wraps it in another function that performs the replacement on its return value.

@admirsaheta admirsaheta requested a review from a team as a code owner August 4, 2024 09:06
@admirsaheta
Copy link
Contributor Author

Can we merge? @craigbrunner

@jesstelford
Copy link
Contributor

@admirsaheta You'll need to add a Changeset before we can merge. Here's one I wrote up for you that you should be able to commit to your branch: https://github.com/Shopify/quilt/blob/680d20e75a1e3cab3a3206f9af90534ada9baade/.changeset/spicy-panthers-prove.md?plain=1

@jesstelford jesstelford force-pushed the fix/loader-module-webpack branch from 6d92aea to 4906ef1 Compare September 13, 2024 06:54
@admirsaheta admirsaheta force-pushed the fix/loader-module-webpack branch from 4906ef1 to 7f346a9 Compare September 13, 2024 19:05
@admirsaheta
Copy link
Contributor Author

Done, also added a check to avoid this from happening @jesstelford
image

@jesstelford jesstelford force-pushed the fix/loader-module-webpack branch from 7f346a9 to 4638287 Compare September 24, 2024 05:42
@jesstelford jesstelford merged commit 525342f into Shopify:main Sep 24, 2024
5 checks passed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants