Skip to content

Cleanup shared configs#62

Open
Pearce-Ropion wants to merge 2 commits into
mainfrom
claude/bold-thompson-FvgXu
Open

Cleanup shared configs#62
Pearce-Ropion wants to merge 2 commits into
mainfrom
claude/bold-thompson-FvgXu

Conversation

@Pearce-Ropion
Copy link
Copy Markdown
Collaborator

@Pearce-Ropion Pearce-Ropion commented May 22, 2026

Summary

Reverses direction from the original "move __VERSION__ into root base configs" — instead, all @sigmacomputing/plugin-specific settings are pulled out of the shared root configs and pushed into packages/plugin-sdk/. The root base configs now hold only settings that genuinely apply to every workspace package.

Moved out of root base configs into packages/plugin-sdk/:

  • tsdown.base.tspackages/plugin-sdk/tsdown.config.ts: __VERSION__ define, inputOptions.transform.jsx: 'react', and the UMD outputOptions (entryFileNames, globals.react, name).
  • vitest.base.tspackages/plugin-sdk/vitest.config.ts: __VERSION__ define, oxc.jsx, and the react/jsx-dev-runtime optimizeDeps entry.
  • tsconfig.app.jsonpackages/plugin-sdk/tsconfig.app.json: "jsx": "react".

Other tightening in the root bases:

  • vitest.base.ts test include narrowed from src/**/*.test.{js,jsx,ts,tsx} to src/**/*.test.{ts,tsx} (no JS/JSX test files in workspace packages).

Test plan

  • yarn turbo build succeeds and the UMD/ESM/CJS bundles still embed the correct version at __VERSION__ (consumed in packages/plugin-sdk/src/client/initialize.ts:54) and produce sigmacomputing-plugin.umd.js with the SigmaPlugin global.
  • yarn turbo test passes — __VERSION__, JSX transform, and react/jsx-dev-runtime resolution still work from the package-local vitest config.
  • yarn turbo types / tsc -b passes — jsx: "react" is correctly picked up from the package's tsconfig.app.json.

@Pearce-Ropion Pearce-Ropion requested a review from a team May 22, 2026 00:46
@Pearce-Ropion Pearce-Ropion marked this pull request as ready for review May 22, 2026 00:46
Copy link
Copy Markdown
Contributor

@rilescode rilescode left a comment

Choose a reason for hiding this comment

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

Would this be an issue if someone manually updates the version in plugin-sdk/package.json without using yarn bump? They could get out of sync

@peternandersson
Copy link
Copy Markdown
Contributor

I'd rather have this just in the plugin-sdk package since it's the only one that needs the value injected

@Pearce-Ropion
Copy link
Copy Markdown
Collaborator Author

I'd rather have this just in the plugin-sdk package since it's the only one that needs the value injected

This is prep work for splitting the package into multiple sub-packages. Should each one inject its own version thus requiring duplicated code for each package's tsdown config?

Would this be an issue if someone manually updates the version in plugin-sdk/package.json without using yarn bump? They could get out of sync

This would generally be a problem and that code should not be landed. The whole package should remain in sync.

The root package.json and all workspace packages share the same version,
so defining __VERSION__ once in tsdown.base.ts and vitest.base.ts removes
the per-package override boilerplate.
@Pearce-Ropion Pearce-Ropion force-pushed the claude/bold-thompson-FvgXu branch from c790ad4 to f640036 Compare May 23, 2026 15:50
@Pearce-Ropion
Copy link
Copy Markdown
Collaborator Author

I'd rather have this just in the plugin-sdk package since it's the only one that needs the value injected

Thought about this more and in hindsight, you are totally right. I am going to change this PR to be more focused on cleaning up duplicate configuration in the tsdown config.

@Pearce-Ropion Pearce-Ropion changed the title Move __VERSION__ define into root base configs [tools] Clean up shared configuration files May 23, 2026
@Pearce-Ropion Pearce-Ropion changed the title [tools] Clean up shared configuration files Cleanup shared configs May 23, 2026
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