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

Load audioWorklet in a Webpack 5 compatible way #11845

Closed
wants to merge 5 commits into from

Conversation

Johennes
Copy link
Contributor

@Johennes Johennes commented Nov 9, 2023

For: element-hq/element-web#26229

We currently use worklet-loader but it isn't designed to work with Webpack 5. The built-in worklet support in Webpack 5 requires the use of URLs in the addModule call alongside special configuration.


This change is marked as an internal change (Task), so will not be included in the changelog.

@Johennes Johennes requested a review from a team as a code owner November 9, 2023 15:53
@Johennes Johennes added the T-Task Refactoring, enabling or disabling functionality, other engineering tasks label Nov 9, 2023
@Johennes
Copy link
Contributor Author

Johennes commented Nov 9, 2023

Ah, looks like the dead code scanner doesn't recognize the imports within the worklet as used anymore now. 🤦‍♂️

Will have to look into that.

@Johennes
Copy link
Contributor Author

Johennes commented Nov 9, 2023

I'm actually not sure how to properly fix the dead code issues. The problem is that after switching to loading the worklet using its literal path, we don't require the worklet import anymore but without the import the worklet is considered dead code.

If we could specify the worklet as an entry for the dead code analysis, that should fix it but ts-prune doesn't support specifying entries manually from what I can see (though its successor https://github.com/webpro/knip does).

Alternatively, we could also add ignores for the seemingly unused symbols but that feels whack-a-mole-ish given that we'll have the same problem with web workers once we ditch worker-loader.

@t3chguy any thoughts on how to proceed here?

@t3chguy
Copy link
Member

t3chguy commented Nov 9, 2023

@Johennes could we not trick it by keeping the import and not just not using it with an eslint ignore & ts-ignore for the time being

@Johennes
Copy link
Contributor Author

Johennes commented Nov 9, 2023

That would also work. All of these options feel like they have downsides but maybe this would be the easiest one.

@Johennes
Copy link
Contributor Author

Johennes commented Nov 9, 2023

Upon further testing, turns out that this doesn't actually work. 😔

Screenshot 2023-11-09 at 21 48 02

Guess this will need some more fiddling.

@Johennes Johennes marked this pull request as draft November 9, 2023 20:49
@Johennes
Copy link
Contributor Author

This now works with Webpack 5 but due to the use of import.meta.url doesn't work with Webpack 4 anymore and, like #11860, breaks tests because of the missing mock.

@Johennes Johennes changed the title Use literal path in audioWorklet.addModule for compatibility with Webpack 5 Load audioWorklet in a Webpack 5 compatible way Nov 13, 2023
@Johennes Johennes changed the base branch from develop to johannes/webpack5-workers November 13, 2023 18:37
@Johennes
Copy link
Contributor Author

I'm gonna close this in favor of #11860 in which I have merged this pull request. There is a lot of overlap between these two and both will have to land in concert with element-hq/element-web#26229.

@Johennes Johennes closed this Nov 13, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
T-Task Refactoring, enabling or disabling functionality, other engineering tasks
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants