Skip to content

chore(bson-parsing): integration with compass - #910

Merged
mabaasit merged 7 commits into
timout-long-running-workerfrom
integration-clean-up
Oct 15, 2026
Merged

mabaasit merged 7 commits into
timout-long-running-workerfrom
integration-clean-up

Conversation

@mabaasit

@mabaasit mabaasit commented Oct 5, 2026

Copy link
Copy Markdown
Collaborator

Description

Open Questions

Checklist


Stack created with GitHub Stacks CLI • Give Feedback 💬

@mabaasit
mabaasit added this pull request to stack #894 October 5, 2026 13:01
Copilot AI balanced review requested due to automatic review settings October 5, 2026 13:04
@mabaasit
mabaasit force-pushed the integration-clean-up branch from a141a27 to 504988a Compare October 5, 2026 13:04
@mabaasit mabaasit changed the title chore: integratino with compass change chore(bson-parsing): integration with compass Oct 5, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot review overview

🟡 Changes recommended

The test imports named exports from a worker file that Webpack overwrites with a non-exporting IIFE bundle.

Review effort: Balanced
Findings: 1 High severity

Open (1)
What changed in this PR

Updates the BSON parser worker for Compass compatibility while preserving parser error types.

Changes:

  • Renames worker artifacts to .mjs.
  • Transfers Error objects across the worker boundary.
  • Adds error-type preservation coverage.
File Description
webpack.worker.config.cjs Bundles the worker as worker.mjs.
src/​worker.mts Returns parser errors directly.
src/​worker-types.ts Types worker failures as Error.
src/​worker-client.ts Loads .mjs and rejects with transferred errors.
src/​index.spec.ts Updates worker paths and tests error preservation.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread packages/shell-bson-parser/src/index.spec.ts Outdated
@mabaasit
mabaasit force-pushed the integration-clean-up branch 2 times, most recently from c4bd7b3 to 107e42d Compare October 5, 2026 15:20
@mabaasit
mabaasit force-pushed the integration-clean-up branch from 107e42d to ca1a8ac Compare October 5, 2026 15:47
@mabaasit
mabaasit force-pushed the integration-clean-up branch from ca1a8ac to 8c55a75 Compare October 5, 2026 19:44
@mabaasit
mabaasit force-pushed the integration-clean-up branch from 8c55a75 to 5126e9d Compare October 5, 2026 20:30
@mabaasit
mabaasit force-pushed the integration-clean-up branch from 6ca851c to 4372c96 Compare October 8, 2026 21:42
@mabaasit
mabaasit force-pushed the integration-clean-up branch from 4372c96 to 1af7a1c Compare October 9, 2026 23:03
@mabaasit
mabaasit force-pushed the integration-clean-up branch 2 times, most recently from 75bd63f to 2df8ad5 Compare October 14, 2026 09:52
@mabaasit
mabaasit marked this pull request as ready for review October 14, 2026 10:01
@mabaasit
mabaasit requested review from a team as code owners October 14, 2026 10:01
blobUrl = URL.createObjectURL(new Blob([code], { type: 'text/javascript' }));
// Compass Web injects whatwg-url for bare URL references; it lacks blob APIs.
// Use the native constructor for blob creation and revocation.
blobUrl = globalThis.URL.createObjectURL(

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Is globalThis.URL.createObjectURL always a function?

// On browser env we want to fetch and blob so that the worker
// script can run on the web running locally.
const scriptUrl = new URL('./worker.js', import.meta.url);
if (scriptUrl.protocol === 'file:') {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Should we test this if file: path?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

added in 9c6fcdd

): void {
if (worker) worker.terminate();
if (blobUrl) URL.revokeObjectURL(blobUrl);
if (blobUrl) globalThis.URL.revokeObjectURL(blobUrl);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why are we adding this back?
#891 (comment)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Yes, this showed in the integration with compass-web. There we alias url to whatwg-url and it does not have these methods.

@mabaasit
mabaasit force-pushed the integration-clean-up branch from 9c6fcdd to 75b942a Compare October 15, 2026 10:54
@mabaasit
mabaasit merged commit 2628daa into main Oct 15, 2026
19 checks passed
@mabaasit
mabaasit deleted the integration-clean-up branch October 15, 2026 15:02
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