Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

refactor(utils): move extract-theme to a separate package #1051

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

maiieul
Copy link
Contributor

@maiieul maiieul commented Mar 7, 2025

What is it?

  • Feature / enhancement
  • Bug
  • Docs / tests
  • Other

Why is it needed?

Rollup seems confused by the nx (maybe applies to all monorepos) graph, maybe because qwik-ui uses CJS. We have one generic util package in qwik-ui with the cn helper that we use everywhere and the extractThemeCSS (which uses specificity and therefore css-tree) that we use only in one place. The result with that shared Nx package is that rollup bundles cn with css-tree, and then the q-bundle-graph wants to statically import and therefore prefetch that chunk almost on every other chunks. If I separate those utils into 2 separate Nx packages, rollup doesn't bundle them together anymore, resulting in only cn being imported almost everywhere - which is expected since we use it almost everywhere.

Checklist:

  • My code follows the developer guidelines of this project
  • I have performed a self-review of my own code
  • I have ran pnpm change and documented my changes
  • I have add necessary docs (if needed)
  • Added new tests to cover the fix / functionality

Copy link

changeset-bot bot commented Mar 7, 2025

⚠️ No Changeset found

Latest commit: 19a637d

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@maiieul maiieul force-pushed the move-extract-theme-logic-to-separate-package branch from 8fe6917 to 19a637d Compare March 8, 2025 19:28
Copy link

pkg-pr-new bot commented Mar 8, 2025

Open in Stackblitz

npm i https://pkg.pr.new/qwikifiers/qwik-ui@1051
npm i https://pkg.pr.new/qwikifiers/qwik-ui/@qwik-ui/headless@1051
npm i https://pkg.pr.new/qwikifiers/qwik-ui/@qwik-ui/utils@1051
npm i https://pkg.pr.new/qwikifiers/qwik-ui/@qwik-ui/styled@1051

commit: bb23dc1

@maiieul maiieul force-pushed the move-extract-theme-logic-to-separate-package branch from 19a637d to bb23dc1 Compare March 8, 2025 19:32
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant