Skip to content

feat(worker): simplify input passing#664

Open
avivkeller wants to merge 1 commit intomainfrom
simple-worker-stream
Open

feat(worker): simplify input passing#664
avivkeller wants to merge 1 commit intomainfrom
simple-worker-stream

Conversation

@avivkeller
Copy link
Member

We have no need to pass these inputs twice

@avivkeller avivkeller requested a review from a team as a code owner March 9, 2026 20:05
Copilot AI review requested due to automatic review settings March 9, 2026 20:05
@vercel
Copy link

vercel bot commented Mar 9, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
api-docs-tooling Ready Ready Preview Mar 9, 2026 8:06pm

Request Review

@codecov
Copy link

codecov bot commented Mar 9, 2026

Codecov Report

❌ Patch coverage is 64.28571% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 75.89%. Comparing base (1aef493) to head (4fb9637).
⚠️ Report is 3 commits behind head on main.

Files with missing lines Patch % Lines
src/generators/ast/generate.mjs 0.00% 1 Missing ⚠️
src/generators/jsx-ast/generate.mjs 0.00% 1 Missing ⚠️
src/generators/legacy-html/generate.mjs 0.00% 1 Missing ⚠️
src/generators/legacy-json/generate.mjs 0.00% 1 Missing ⚠️
src/generators/metadata/generate.mjs 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #664      +/-   ##
==========================================
- Coverage   75.90%   75.89%   -0.02%     
==========================================
  Files         145      145              
  Lines       13735    13728       -7     
  Branches      992      992              
==========================================
- Hits        10426    10419       -7     
  Misses       3303     3303              
  Partials        6        6              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Member

@AugustinMauroy AugustinMauroy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGMT ! did this impact perf ?

Copy link
Contributor

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 simplifies the ParallelWorker.stream API by removing the redundant “full input” parameter, updating internal generator call sites, typings, and documentation examples to pass inputs only once.

Changes:

  • Update createParallelWorker().stream to accept (items, extra?) and pass items through to processChunk / worker tasks.
  • Update generator implementations and tests to use the new worker.stream(...) signature.
  • Update TypeScript declarations and docs examples to remove the duplicate input argument.

Reviewed changes

Copilot reviewed 9 out of 10 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
src/threading/parallel.mjs Changes stream signature and updates internal task/processChunk invocation to use items only.
src/threading/tests/parallel.test.mjs Updates most test call sites to the new stream(items, extra?) signature.
src/generators/types.d.ts Updates the ParallelWorker.stream type signature to remove fullInput.
src/generators/metadata/generate.mjs Updates stream invocation to pass inputs once plus typeMap.
src/generators/legacy-json/generate.mjs Updates stream invocation to pass entries once.
src/generators/legacy-html/generate.mjs Updates stream invocation to pass entries once plus navigation.
src/generators/jsx-ast/generate.mjs Updates stream invocation to pass entries once plus docPages.
src/generators/ast/generate.mjs Updates stream invocation to pass files once.
src/generators/ast-js/generate.mjs Updates stream invocation to pass files once.
docs/generators.md Updates example worker.stream(...) calls to the new signature.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@avivkeller
Copy link
Member Author

LGMT ! did this impact perf ?

I doubt it. Inside of stream we are really only looking at the length of items and the content of fullInput, and they are the same object.

Copy link
Member

@ovflowd ovflowd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The PR seems to be simplifying things, but please check all generator outputs and compare them before after this change with actuall file hash diffs. Want to be extra sure this is not really changing anything :P

@avivkeller
Copy link
Member Author

please check all generator outputs and compare them before after this change with actuall file hash diffs. Want to be extra sure this is not really changing anything :P

It's not. I've checked locally, plus, if there was a change, our GitHub Actions would've left a comment on the PR

@ovflowd
Copy link
Member

ovflowd commented Mar 9, 2026

please check all generator outputs and compare them before after this change with actuall file hash diffs. Want to be extra sure this is not really changing anything :P

It's not. I've checked locally, plus, if there was a change, our GitHub Actions would've left a comment on the PR

OBEY 😆

@ovflowd
Copy link
Member

ovflowd commented Mar 9, 2026

anyhow, sgtm :D

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.

4 participants