Skip to content

chore(ui): use compiled exports by default - #8626

Closed
avivkeller wants to merge 2 commits into
mainfrom
fix-ui-components-conditions
Closed

avivkeller wants to merge 2 commits into
mainfrom
fix-ui-components-conditions

Conversation

@avivkeller

Copy link
Copy Markdown
Member

It makes little sense to me why the version not published to npm is the default. It should be the other way around.

@avivkeller
avivkeller requested a review from a team as a code owner February 13, 2026 19:17
Copilot AI review requested due to automatic review settings February 13, 2026 19:17
@avivkeller
avivkeller requested a review from a team as a code owner February 13, 2026 19:17
@vercel

vercel Bot commented Feb 13, 2026 •

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
nodejs-org Error Error Feb 13, 2026 7:23pm

Request Review

@github-actions

Copy link
Copy Markdown
Contributor

👋 Codeowner Review Request

The following codeowners have been identified for the changed files:

Team reviewers: @nodejs/web-infra @nodejs/nodejs-website

Please review the changes when you have a chance. Thank you! 🙏

@codecov

codecov Bot commented Feb 13, 2026 •

Copy link
Copy Markdown

❌ 5 Tests Failed:

Tests completed Failed Passed Skipped
74 5 69 0
View the top 3 failed test(s) by shortest run time
test::src/Containers/MetaBar/__tests__/index.test.jsx
Stack Traces | 3.68s run time
[Error: test failed] { code: 'ERR_TEST_FAILURE', failureType: 'testCodeFailure', cause: 'test failed', exitCode: 7, signal: null }
test::src/Common/BasePagination/PaginationListItem/__tests__/index.test.jsx
Stack Traces | 4.61s run time
[Error: test failed] { code: 'ERR_TEST_FAILURE', failureType: 'testCodeFailure', cause: 'test failed', exitCode: 7, signal: null }
test::src/Common/BasePagination/__tests__/index.test.jsx
Stack Traces | 4.81s run time
[Error: test failed] { code: 'ERR_TEST_FAILURE', failureType: 'testCodeFailure', cause: 'test failed', exitCode: 7, signal: null }
test::src/Common/AvatarGroup/__tests__/index.test.jsx
Stack Traces | 5.04s run time
[Error: test failed] { code: 'ERR_TEST_FAILURE', failureType: 'testCodeFailure', cause: 'test failed', exitCode: 7, signal: null }
test::src/Common/Select/__tests__/index.test.jsx
Stack Traces | 5.53s run time
[Error: test failed] { code: 'ERR_TEST_FAILURE', failureType: 'testCodeFailure', cause: 'test failed', exitCode: 7, signal: null }

To view more test analytics, go to the Test Analytics Dashboard
📋 Got 3 mins? Take this short survey to help us improve Test Analytics.

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.

Pull request overview

This PR updates @node-core/ui-components to prefer compiled (dist) entrypoints by default (matching what’s published to npm), and adjusts publishing to run the compile step explicitly.

Changes:

  • Swap package.json exports/imports conditions so default resolves to dist and add an uncompiled condition for src.
  • Update packages/ui-components/tsconfig.json to resolve using the uncompiled condition.
  • Update the publish workflow to run pnpm run compile (and remove the package-level release script).

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

File Description
packages/ui-components/tsconfig.json Changes TS module resolution conditions to prefer uncompiled.
packages/ui-components/package.json Makes compiled dist the default for both exports and internal imports, adds uncompiled condition targets, removes release script.
.github/workflows/publish-packages.yml Runs compile (instead of release) prior to pnpm publish when available.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 6 to 11
"./*": {
"rolldown": [
"default": [
"./dist/*",
"./dist/*.js",
"./dist/*/index.js"
],

Copilot AI Feb 13, 2026

Copy link

Choose a reason for hiding this comment

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

Switching the exports default condition to ./dist/* will make workspace consumers (e.g. apps/site) resolve @node-core/ui-components/... to dist, but dist/ is not present in the repo and this package does not currently participate in the Turbo build/dev pipeline. As-is, local dev/CI builds that import @node-core/ui-components/... are likely to fail unless pnpm run compile is executed beforehand. Consider either ensuring compile runs automatically before consumers build/dev (e.g. add a build script + Turbo task outputs for dist/** and make dependent tasks depend on it), or keep the workspace-default pointing at src and only use dist as default for published artifacts via conditions/publish config.

Copilot uses AI. Check for mistakes.
"default": [
"./dist/*",
"./dist/*.js",
"./dist/*/index.js"

Copilot AI Feb 13, 2026

Copy link

Choose a reason for hiding this comment

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

imports["#ui/*"].default now points to ./dist/*, but a lot of source files (and Storybook preview) import #ui/... while running directly from src (without a preceding compile step). Since dist/ isn't present by default, Storybook/tests/dev in this repo are likely to break unless you either (1) run compile before those tasks, or (2) configure the relevant runners/bundlers to resolve the uncompiled condition (e.g. Node --conditions=uncompiled, and webpack/Storybook resolve.conditionNames including uncompiled).

Suggested change
"./dist/*/index.js"
"./dist/*/index.js",
"./src/*",
"./src/*.tsx",
"./src/*/index.tsx",
"./src/*.ts",
"./src/*/index.ts"

Copilot uses AI. Check for mistakes.
Comment thread .github/workflows/publish-packages.yml

@ovflowd ovflowd left a comment

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.

Might not make sense to you, but I've done this after several testing, the other order, things break, hard.

@ovflowd

ovflowd commented Feb 13, 2026

Copy link
Copy Markdown
Member

(as you noticed yourself)

@avivkeller avivkeller closed this Feb 13, 2026
@avivkeller

Copy link
Copy Markdown
Member Author

I see. Had just run compile before testing, so my terminal didn't report any errors :-/

@ovflowd

ovflowd commented Feb 13, 2026

Copy link
Copy Markdown
Member

I see. Had just run compile before testing, so my terminal didn't report any errors :-/

I can explain via Slack, why I had to make this way if it helps 🫠

This branch had an error being deployed

1 failed deployment
Preview — 1735d5f2 Deployed Feb 13, 2026 by vercel[bot]
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.

3 participants