Skip to content

Bazel js toolchain rewrite#2206

Open
ikholopov-omni wants to merge 11 commits into
dataform-co:mainfrom
ikholopov-omni:bazel7js
Open

Bazel js toolchain rewrite#2206
ikholopov-omni wants to merge 11 commits into
dataform-co:mainfrom
ikholopov-omni:bazel7js

Conversation

@ikholopov-omni

@ikholopov-omni ikholopov-omni commented Jun 16, 2026

Copy link
Copy Markdown
Collaborator
  • Migrate to maintained aspect-rules for JS toolchain
  • Use pnpm instead of yarn, remove no-longer required scripts for switching registry (as pnpm-lock doesn't hard-code the domain, it always respects .npmrc config)
  • Keep the same bundling layout - ESM for CLI, CommonJS for Core
  • Updated scripts to use updated tooling
  • Complete migration to BzlMod, cleanup WORKSPACE

@ikholopov-omni ikholopov-omni force-pushed the bazel7js branch 5 times, most recently from 379cf44 to 665c009 Compare June 17, 2026 13:53
@ikholopov-omni ikholopov-omni marked this pull request as ready for review June 17, 2026 14:32
@ikholopov-omni ikholopov-omni requested a review from a team as a code owner June 17, 2026 14:32
Comment thread packages/rollup.config.js Outdated

@kolina kolina 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.

It makes sense to manually check that it doesn't break GCP compilation

mode: argv.mode || "development",
target: 'node',
entry: [path.resolve(runfilesDir, workspaceName, "packages/@dataform/core/index")],
mode: argv.mode || "production",

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.

is changing default here needed?

Comment thread packages/rollup.config.js
if (pattern instanceof RegExp) {
return pattern;
}
// If it's a string, turn it into a regex, by escaping any regex characters in the string.

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.

AI loves to remove comments :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants