Skip to content

feat(#110): adop vite build migration for all packages#213

Open
saint0x wants to merge 2 commits into
briehq:developfrom
saint0x:vite-build
Open

feat(#110): adop vite build migration for all packages#213
saint0x wants to merge 2 commits into
briehq:developfrom
saint0x:vite-build

Conversation

@saint0x

@saint0x saint0x commented Jun 13, 2025

Copy link
Copy Markdown

Purpose of the PR*

Migrated all packages to use a unified Vite build workflow and improved TypeScript declarations and configuration consistency across packages.

Priority*

  • High: This PR needs to be merged first, before other tasks.
  • Medium: This PR should be merged quickly to prevent conflicts due to common changes. (default)
  • Low: This PR does not affect other tasks, so it can be merged later.

Changes*

  1. Migrated all packages to the Vite build system.
  2. Introduced withLibraryConfig in each package's Vite config for consistency.
  3. Removed .js suffixes in re-exports and aligned package.json types fields with generated .d.(m)ts files.
  4. Enabled --skipLibCheck and added a shared packages/tsconfig/library.json to fix type declaration build issues.
  5. Refactored vite-config/lib/withLibraryConfig.ts to remove @extension/env and eliminate circular HMR/env dependency.
  6. Fixed zipper top-level await issue by explicitly targeting node18.
  7. Added Slice<…> generics to all Redux slices for improved type safety.
  8. Removed old tsconfig/dist copy workaround.
  9. Cleaned up miscellaneous any type annotations.

How to check the feature / Video Demo

Run pnpm build in each package and confirm:

  • Declarations are generated correctly.
  • Vite config works uniformly.
  • No circular deps or HMR issues.
  • Redux slices are properly typed.

Reference

  • This update addresses multiple long-standing inconsistencies across the monorepo.
  • Helps simplify maintenance and improves type safety and tooling compatibility going forward.

@saint0x saint0x requested a review from ionleu as a code owner June 13, 2025 11:04
@github-actions

Copy link
Copy Markdown

Your PR was set to target main, but it should be directed to develop. The base branch has been updated to develop, please check for any merge conflicts.

@github-actions github-actions Bot requested a review from LuminitaL June 13, 2025 11:04
@github-actions github-actions Bot changed the base branch from main to develop June 13, 2025 11:04
@github-actions

Copy link
Copy Markdown

Hey @saint0x,
Thanks for another PR! 🚀

@@ -1 +1 @@
export * from './lib/index.js';
export * from './lib/index';

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.

Are we not able to use this without the index path part?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

yes, since everything is now standardized it could be removed if you prefer -- kept it for explicitness

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.

Let's remove the index path from everywhere it still used.

Comment thread packages/dev-utils/package.json
Comment thread packages/dev-utils/tsconfig.json
Comment thread packages/env/package.json
"@extension/tsconfig": "workspace:*"
"@extension/tsconfig": "workspace:*",
"@extension/vite-config": "workspace:*",
"vite": "6.1.0"

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.

I think we should be good without installing vite in every package, since it is installed in the root. Or I'm wrong?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

the explicit references are necessary for individual package builds (if just working on one in isolation, for example)

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.

So if we'll use the root vite project the changes from here will not work properly?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

it will still work properly, just will be an extra step

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.

I'd prefer we use the one from the root. Will this add complexity to implement?

Comment thread packages/hmr/index.mts
@@ -1 +1 @@
export * from './lib/plugins/index.js';
export * from './lib/plugins/index';

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.

Still required to use index?

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.

As discussed above, we'll remove it

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

kk, got it

Comment thread packages/store/index.mts
export { ReduxProvider };
export * from './lib/store/index.js';
export * from './lib/hooks/index.js';
export * from './lib/store/index';

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.

Same as other imports from above, still needs index?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

yep, same as others: explicit references are typically better to prevent potential future circular dependencies as well -- but if you don't like it, technically it could be removed.

@@ -1 +1 @@
export * from './lib/index.js';
export * from './lib/index';

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.

needs index?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

see above ^

@ionleu ionleu changed the title clean vite build migration for all packages feat(#110): adop vite build migration for all packages Jun 14, 2025
@ionleu ionleu added improvements Improves the existing feature refactor Refactors the existing feature feature New feature or request devex Improve developers experience labels Jun 14, 2025
@ionleu

ionleu commented Jun 14, 2025

Copy link
Copy Markdown
Contributor

@saint0x,
Can you please test if run, build and zip scripts are working as expected?

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

Labels

devex Improve developers experience feature New feature or request improvements Improves the existing feature refactor Refactors the existing feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Adopt tsup, esbuild or vite for Cleaner ESM Builds and Extensionless Imports

2 participants