Skip to content

feat(ui-components): prepublish #7955

New issue

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

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

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Jul 7, 2025
Merged

feat(ui-components): prepublish #7955

merged 5 commits into from
Jul 7, 2025

Conversation

avivkeller
Copy link
Member

@avivkeller avivkeller commented Jul 5, 2025

This PR adds compilation to a pre-publish script for @node-core/ui-components. The way this works is:

  1. CI runs pnpm run publish
  2. The publish script:
    1. Compiles all .tsx-like files to their JS counterpart in dist/
    2. Compiles all Tailwind CSS modules to their pure CSS counterparts in dist/
    3. Adds a package.json file to ./dist, rewriting the exports and imports.
  3. pnpm publishes dist/

Fixes #7948

@Copilot Copilot AI review requested due to automatic review settings July 5, 2025 23:38
@avivkeller avivkeller requested review from a team as code owners July 5, 2025 23:38
Copy link

vercel bot commented Jul 5, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
nodejs-org ✅ Ready (Inspect) Visit Preview Jul 7, 2025 8:14pm

@avivkeller avivkeller added the github_actions:pull-request Trigger Pull Request Checks label Jul 5, 2025
@github-actions github-actions bot removed the github_actions:pull-request Trigger Pull Request Checks label Jul 5, 2025
@avivkeller
Copy link
Member Author

oookay so it looks like something went horribly wrong

Copy link
Contributor

@Copilot Copilot AI left a comment

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 implements a pre-publish build for @node-core/ui-components by configuring TypeScript and CSS compilation, updating module imports, and packaging the output.

  • Configure tsconfig.json to emit compiled JS into dist/ with correct root and base directories.
  • Update all CSS modules to include @reference directives and adjust @source/@layer usage.
  • Replace #ui/* alias imports with relative paths and add prepublish scripts plus a clean package.json writer.

Reviewed Changes

Copilot reviewed 96 out of 97 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
packages/ui-components/tsconfig.json Set rootDir, outDir, baseUrl, and adjust compilerOptions for dist output
packages/ui-components/src/styles/*.css Add @reference directives, consolidate @source imports, and wrap utilities layers
packages/ui-components/src/**/*.tsx Replace custom alias imports (#ui/...) with relative imports
packages/ui-components/scripts/prepublish.mjs Write a cleaned package.json into dist/ without devDependencies
packages/ui-components/package.json Add compile:ts, compile:css, and prepublish scripts; move CSS deps to devDependencies
Files not reviewed (1)
  • pnpm-lock.yaml: Language not supported
Comments suppressed due to low confidence (1)

packages/ui-components/package.json:17

  • The node --run compile command is not a valid Node.js CLI flag. Consider changing this to npm run compile (or pnpm run compile) to invoke the compile script correctly.
    "prepublish": "node --run compile && node scripts/prepublish.mjs",

@avivkeller avivkeller marked this pull request as draft July 5, 2025 23:41
@ovflowd
Copy link
Member

ovflowd commented Jul 5, 2025

FYI Vercel failure was due to:

"publishDirectory" in the lockfile (undefined) doesn't match "publishConfig.directory" in package.json (dist)

@avivkeller
Copy link
Member Author

@ovflowd Yea, I'm aware, I didn't sync the lockfile

Copy link

codecov bot commented Jul 6, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 73.66%. Comparing base (ab0ba1d) to head (ed0e5d8).
Report is 4 commits behind head on main.

✅ All tests successful. No failed tests found.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #7955      +/-   ##
==========================================
- Coverage   75.36%   73.66%   -1.70%     
==========================================
  Files          96       96              
  Lines        8354     8354              
  Branches      220      220              
==========================================
- Hits         6296     6154     -142     
- Misses       2056     2199     +143     
+ Partials        2        1       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@avivkeller avivkeller marked this pull request as ready for review July 6, 2025 00:11
Copy link
Member

@ovflowd ovflowd left a comment

Choose a reason for hiding this comment

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

LGTM. I guess we can propceed with this :)

@ovflowd ovflowd added the github_actions:pull-request Trigger Pull Request Checks label Jul 6, 2025
@github-actions github-actions bot removed the github_actions:pull-request Trigger Pull Request Checks label Jul 6, 2025
@avivkeller
Copy link
Member Author

@avivkeller avivkeller marked this pull request as draft July 6, 2025 15:30
@avivkeller
Copy link
Member Author

avivkeller commented Jul 6, 2025

I think I know how to fix the issue + reduce the number of changed lines. Drafting until I get the time to properly implement the changes.

@avivkeller avivkeller added the github_actions:pull-request Trigger Pull Request Checks label Jul 7, 2025
@github-actions github-actions bot removed the github_actions:pull-request Trigger Pull Request Checks label Jul 7, 2025
@avivkeller avivkeller marked this pull request as ready for review July 7, 2025 17:43
@avivkeller
Copy link
Member Author

I'd like to get a second @nodejs/web-infra approval before landing this JiC fwiw.

@ovflowd
Copy link
Member

ovflowd commented Jul 7, 2025

I'd like to get a second @nodejs/web-infra approval before landing this JiC fwiw.

Lots of stuff failing, should we retry these checks?

@ovflowd ovflowd added the github_actions:pull-request Trigger Pull Request Checks label Jul 7, 2025
@github-actions github-actions bot removed the github_actions:pull-request Trigger Pull Request Checks label Jul 7, 2025
Copy link
Member

@ovflowd ovflowd left a comment

Choose a reason for hiding this comment

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

LGTM

Signed-off-by: Aviv Keller <[email protected]>
@ovflowd
Copy link
Member

ovflowd commented Jul 7, 2025

@avivkeller tests/linting failing?

@avivkeller
Copy link
Member Author

Yes, I know. I need to change some rootDir TSConfig things.

@avivkeller avivkeller added the github_actions:pull-request Trigger Pull Request Checks label Jul 7, 2025
@github-actions github-actions bot removed the github_actions:pull-request Trigger Pull Request Checks label Jul 7, 2025
@ovflowd ovflowd added this pull request to the merge queue Jul 7, 2025
Merged via the queue into main with commit b44cc18 Jul 7, 2025
17 checks passed
@ovflowd ovflowd deleted the feat/ui/prepublish branch July 7, 2025 20:24
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.

Build @node-core/ui-components before publishing
2 participants