-
Notifications
You must be signed in to change notification settings - Fork 205
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
feat: run tasks in parallel with workers #792
base: main
Are you sure you want to change the base?
Conversation
053f3d7
to
28a0f78
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR implements parallel task execution using node worker threads to speed up target builds.
- Introduces a new workerize utility to encapsulate worker thread management.
- Updates codegen patch error messages for improved clarity.
- Refactors target execution for module, typescript, codegen, and custom to use the workerize.run function.
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
File | Description |
---|---|
packages/react-native-builder-bob/src/utils/workerize.ts | New file providing worker thread logic to run targets in parallel. |
packages/react-native-builder-bob/src/targets/codegen/patches/patchCodegenAndroidPackage.ts | Enhances error messages by displaying relative paths using kleur. |
packages/react-native-builder-bob/src/index.ts | Migrates several target invocations to use workerize.run for parallel execution, while the commonjs target remains unchanged. |
Comments suppressed due to low confidence (1)
packages/react-native-builder-bob/src/index.ts:531
- The commonjs target still directly invokes buildCommonJS instead of using run, which creates an inconsistency in how targets are executed in parallel. Consider updating the commonjs case to also use run for consistency.
await buildCommonJS({
28a0f78
to
edc6e71
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR refactors the build process to run tasks in parallel using Node workers instead of sequential function calls.
- Introduces a new worker module (workerize.ts) to abstract the worker thread creation and communication
- Updates the build script to invoke targets via the new worker mechanism, while also adding colored output in the codegen patch
- Removes traditional target-specific builds in favor of the worker-based approach
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
File | Description |
---|---|
packages/react-native-builder-bob/src/utils/workerize.ts | Implements worker thread setup and error handling for parallel target processing |
packages/react-native-builder-bob/src/targets/codegen/patches/patchCodegenAndroidPackage.ts | Enhances console output using colored paths for better messaging |
packages/react-native-builder-bob/src/build.ts | Replaces direct calls to build functions with worker invocations for several targets |
Comments suppressed due to low confidence (2)
packages/react-native-builder-bob/src/build.ts:144
- The 'commonjs' target handling has been removed without a corresponding call to run the worker. If running targets in parallel is intended for all cases, please add a run invocation for 'commonjs'.
case 'commonjs':
packages/react-native-builder-bob/src/utils/workerize.ts:88
- Consider replacing console.log(error) with report.error(error.message) for consistent error handling, and ensure that process.exit(1) is the desired behavior in this context.
console.log(error);
d736fc1
to
12dad46
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR refactors the build process to run targets in parallel using Node.js worker threads. Key changes include:
- Introducing a new worker-based implementation for executing target builds (workerize.ts).
- Updating target build invocations in build.ts to dispatch work via the new worker mechanism.
- Enhancing error message output in the codegen patch file by colorizing paths with kleur.
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
File | Description |
---|---|
packages/react-native-builder-bob/src/utils/workerize.ts | Implements worker thread handling for running target builds in parallel. |
packages/react-native-builder-bob/src/targets/codegen/patches/patchCodegenAndroidPackage.ts | Updates error messaging to use kleur for improved clarity. |
packages/react-native-builder-bob/src/build.ts | Refactors target build invocations to use the workerized run function. |
Comments suppressed due to low confidence (2)
packages/react-native-builder-bob/src/build.ts:145
- The dedicated block for the 'commonjs' target has been removed. For consistency with other targets and to ensure that the commonjs build is executed in a worker thread, consider replacing it with a call to run('commonjs', { ... }).
case 'commonjs':
packages/react-native-builder-bob/src/utils/workerize.ts:87
- [nitpick] In the worker thread, errors are currently logged using console.log before exiting. To maintain consistent error reporting, consider using report.error(error.message) to relay the error back to the parent process.
targets[target]({ ...data, report }).catch((error) => {
12dad46
to
164dd62
Compare
Summary
This makes the targets run in parallel using node workers instead of concurrently.
Test plan
Test building in a fresh project.