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

Revert "fix(webpack-config): fix sourcemap bug with expo camera" #4746

Closed

Conversation

marklawlor
Copy link
Contributor

Reverts #4735

We are removing @koale/useworker so these rules can be moved

expo/expo#23967

@EvanBacon
Copy link
Contributor

Probably ok to keep these around for a while since the webpack config isn't versioned with the SDK.

EvanBacon pushed a commit to expo/expo that referenced this pull request Aug 16, 2023
# Why

`@koale/useworker` is only used for Web QR code scanning and has been
causing some issues due to lack of maintenance.

Fixes: #17846
Related: #23296, and the PR to revert
its 'fix' expo/expo-cli#4746

# How

Refactors the existing code to use vanilla JS

1. We create a inline webworker by creating an inline blob with
`qrWorkerMethod.toString()` (code adapted from `@koale/useworker`)
2. Create a smaller wrapper that tracks messages being sent to the
worker via a queue of promises

As the worker is synchronous and will receive messages in order a simple
First In First Out queue should suffice.

This does _slightly_ change the functionality of the existing code. 

1. Previously we created a new Worker for every instance of `<Camera
/>`. Now a single worker is shared across all instances.
1. As the worker is declared in the global scope, it is created
regardless if QR code scanning is enabled. We could lazy initialise it,
but because its extremely small and inline (no network requests) I don't
see this being an issue.

I also removed an extra `useEffect` hook by moving its logic into the
cleanup step of the enabling hook.
EvanBacon pushed a commit to expo/expo that referenced this pull request Aug 23, 2023
# Why

`@koale/useworker` is only used for Web QR code scanning and has been
causing some issues due to lack of maintenance.

Fixes: #17846
Related: #23296, and the PR to revert
its 'fix' expo/expo-cli#4746

# How

Refactors the existing code to use vanilla JS

1. We create a inline webworker by creating an inline blob with
`qrWorkerMethod.toString()` (code adapted from `@koale/useworker`)
2. Create a smaller wrapper that tracks messages being sent to the
worker via a queue of promises

As the worker is synchronous and will receive messages in order a simple
First In First Out queue should suffice.

This does _slightly_ change the functionality of the existing code. 

1. Previously we created a new Worker for every instance of `<Camera
/>`. Now a single worker is shared across all instances.
1. As the worker is declared in the global scope, it is created
regardless if QR code scanning is enabled. We could lazy initialise it,
but because its extremely small and inline (no network requests) I don't
see this being an issue.

I also removed an extra `useEffect` hook by moving its logic into the
cleanup step of the enabling hook.
brentvatne pushed a commit to expo/expo that referenced this pull request Sep 7, 2023
`@koale/useworker` is only used for Web QR code scanning and has been
causing some issues due to lack of maintenance.

Fixes: #17846
Related: #23296, and the PR to revert
its 'fix' expo/expo-cli#4746

Refactors the existing code to use vanilla JS

1. We create a inline webworker by creating an inline blob with
`qrWorkerMethod.toString()` (code adapted from `@koale/useworker`)
2. Create a smaller wrapper that tracks messages being sent to the
worker via a queue of promises

As the worker is synchronous and will receive messages in order a simple
First In First Out queue should suffice.

This does _slightly_ change the functionality of the existing code.

1. Previously we created a new Worker for every instance of `<Camera
/>`. Now a single worker is shared across all instances.
1. As the worker is declared in the global scope, it is created
regardless if QR code scanning is enabled. We could lazy initialise it,
but because its extremely small and inline (no network requests) I don't
see this being an issue.

I also removed an extra `useEffect` hook by moving its logic into the
cleanup step of the enabling hook.
@byCedric byCedric added question Further information is requested expo:internal-discussion PRs that require Expo internal discussions and removed question Further information is requested labels Dec 22, 2023
@byCedric byCedric removed the expo:internal-discussion PRs that require Expo internal discussions label Jan 4, 2024
@byCedric
Copy link
Member

byCedric commented Jan 5, 2024

Closing this due to inactivity. @expo/webpack-config has been moved to https://github.com/expo/expo-webpack-integrations.

@byCedric byCedric closed this Jan 5, 2024
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