-
-
Notifications
You must be signed in to change notification settings - Fork 6.4k
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
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
oookay so it looks like something went horribly wrong |
There was a problem hiding this 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 intodist/
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 cleanpackage.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 tonpm run compile
(orpnpm run compile
) to invoke thecompile
script correctly.
"prepublish": "node --run compile && node scripts/prepublish.mjs",
FYI Vercel failure was due to:
|
@ovflowd Yea, I'm aware, I didn't sync the lockfile |
5e124cd
to
ba211da
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
✅ 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. |
There was a problem hiding this 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 :)
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. |
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? |
There was a problem hiding this 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]>
Signed-off-by: Aviv Keller <[email protected]>
@avivkeller tests/linting failing? |
Yes, I know. I need to change some rootDir TSConfig things. |
This PR adds compilation to a pre-publish script for
@node-core/ui-components
. The way this works is:pnpm run publish
publish
script:.tsx
-like files to their JS counterpart indist/
dist/
package.json
file to./dist
, rewriting the exports and imports.dist/
Fixes #7948