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

feat: run tasks in parallel with workers #792

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

satya164
Copy link
Member

Summary

This makes the targets run in parallel using node workers instead of concurrently.

CleanShot 2025-03-15 at 19 48 51@2x

Test plan

Test building in a fresh project.

@satya164 satya164 requested a review from atlj March 15, 2025 18:52
@satya164 satya164 force-pushed the @satya164/parallel branch 4 times, most recently from 053f3d7 to 28a0f78 Compare March 15, 2025 19:45
@satya164 satya164 requested a review from Copilot March 27, 2025 13:09
Copy link

@Copilot Copilot AI left a 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({

@satya164 satya164 force-pushed the @satya164/parallel branch from 28a0f78 to edc6e71 Compare March 27, 2025 13:21
@satya164 satya164 requested a review from Copilot March 27, 2025 13:21

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);
@satya164 satya164 force-pushed the @satya164/parallel branch 3 times, most recently from d736fc1 to 12dad46 Compare March 27, 2025 13:22
@satya164 satya164 requested a review from Copilot March 27, 2025 13:23

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) => {
@satya164 satya164 force-pushed the @satya164/parallel branch from 12dad46 to 164dd62 Compare March 27, 2025 13:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant