-
Notifications
You must be signed in to change notification settings - Fork 4k
ui: remove "oss" build cruft #158910
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
Open
dhartunian
wants to merge
5
commits into
cockroachdb:master
Choose a base branch
from
dhartunian:davidh/push-wqwkmyonzrsv
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
ui: remove "oss" build cruft #158910
dhartunian
wants to merge
5
commits into
cockroachdb:master
from
dhartunian:davidh/push-wqwkmyonzrsv
+23,224
−18,334
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
In order to upgrade our versions of node, pnpm, webpack, etc. we first need to update our bazel build plugins. We want to support Node 20+, and pnpm 9+. pnpm 9 requires `rules_js` 2.x. `rules_nodejs` 6.1.0+ is required by `rules_js` 2.x. This also forces an upgrade of `rules_ts`. WORKSPACE Updates - rules_js: 1.42.3 → 2.3.7 - rules_ts: 1.4.0 → 3.4.0 - rules_nodejs: 5.8.2 → 6.3.2 - aspect_bazel_lib: 1.42.3 → 2.9.4 - bazel_features: 0.2.0 → 1.23.0 - rules_jest: 0.18.4 → 0.22.0 - Added new toolchain registrations (yq, tar) which bazel complained about - Added bazel_features_deps() call .bazelrc Updates These are required with the new rules_ts version. - Added --@aspect_rules_ts//ts:skipLibCheck=honor_tsconfig - Added --@aspect_rules_ts//ts:default_to_tsc_transpiler BUILD.bazel Updates - cluster-ui: Changed supports_workers = False to supports_workers = 0. This was a breaking chjange in `rules_ts`. - Added pkg aliases to all 5 npm_package targets (rules_js 2.x breaking change) build/nodejs.bzl Updates - Updated load path from toolchains_repo to nodejs_toolchains_repo. This was a rename in the latest `rules_nodejs`. Epic: None Release note: None ---- TODO Before Merging - [ ] Mirror the tarballs to GCS (rules_js, rules_ts, rules_nodejs, bazel-lib, bazel_features, rules_jest) - [ ] Run build/scripts/build-bazel-lib-helpers.sh to regenerate copy_directory/copy_to_directory binaries for bazel-lib 2.x - [ ] Update URLs in WORKSPACE from GitHub to GCS mirrors
22.11.0 is the first LTS release of node 22. pkg/cmd/dev/ui.go - Modified CLI args to pass env vars to webpack due to API change. pkg/cmd/mirror/npm/main.go - pnpm changed the dep name structure so mirroring URL construction needed a tweak. There's no longer a leading `/` pkg/ui/package.json - add `onlyBuiltDependencies` configuration for pnpm since the new version requires allowlisting any post-build scripts. pkg/ui/workspaces/cluster-ui - webpack config adjustments. polyfill for "path" usage, which is no longer automatically provided, and changed url-loader to use "asset" type which is natively supported by webpack 5. pkg/ui/workspaces/db-console - webpack config adjustments. polyfills for various node libraries, which are no longer automatically provided. buffer and process now need to be provided via the `ProvidePlugin` since those are used without explicit import. The rest of the changes are just plugin syntax. Epic: None Release note: None
This was automated with Claude. - Converted 98 .styl files to .scss (0 remaining) - Used https://www.npmjs.com/package/stylus-converter for automated conversion - Fixed post-conversion issues: - Stylus placeholder syntax ($var {) → SCSS placeholder (%var {) - String interpolation ("calc(100% - %s)" % $var) → SCSS (calc(100% - #{$var})) - Removed nib-specific features, added clearfix mixin - Fixed variable ordering (base colors before derived) - Fixed @extend syntax and double-ampersand selectors - Added missing variable definitions - webpack.config.js - switched from stylus-loader to sass-loader - package.json - replaced stylus/nib/stylint with sass/sass-loader - 98 style files converted from .styl to .scss - All TypeScript imports updated Epic: None Release note: None
Breaking change fixes:
1. cluster-ui/src/dateRangeMenu/dateRangeMenu.tsx:20-43:
- Fixed removed type PickerTimeProps - replaced with PickerProps
from antd + SharedTimeProps from rc-picker for the showNow prop
- Updated TimePicker component to use proper type casting
2. db-console/.../metricsWorkspace.tsx:168:
- Added hasControlInside={false} prop to Upload component (required
in antd 5.20.3's stricter types)
Epic: None
Release note: None
This was automated by Claude. Build Infrastructure: 1. pkg/ui/workspaces/db-console/BUILD.bazel - Removed WEBPACK_DATA_OSS variable and entire db-console-oss target 2. pkg/ui/workspaces/db-console/webpack.config.js - Simplified to always use CCL behavior (no more conditional on env.dist) Gitignore cleanup: 3. pkg/ui/.gitignore - Removed assets.oss.installed and distoss/** exclusions 4. pkg/ui/workspaces/db-console/.gitignore - Removed distoss/distoss.go exclusion Documentation: 5. pkg/ui/ui.go - Updated package comment to remove "OSS UI" reference 6. pkg/ui/README.md - Renamed "CCL Build" section to "Module Resolution" and updated text Variable Renaming: 7. pkg/cmd/dev/ui.go - Renamed protoOss → protoBase and ossDst → baseDst Makefile: 8. GNUmakefile - Removed buildoss from the comment alias example Verification - Go code compiles successfully (go build ./pkg/cmd/dev/...) - ./dev ui --help works correctly - webpack.config.js syntax is valid Not Changed (preserved intentionally) - LicenseType: "OSS" in redux state (license type enum, not build mode) - oss/* path alias in tsconfig.json (import path prefix) - OSSComponent/OSSProps naming (runtime license-based rendering) - import from "oss/src/..." patterns (valid import paths) Epic: None Release note: None
|
Your pull request contains more than 1000 changes. It is strongly encouraged to split big PRs into smaller chunks. 🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf. |
Member
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This was automated by Claude.
Build Infrastructure:
Gitignore cleanup:
3. pkg/ui/.gitignore - Removed assets.oss.installed and distoss/** exclusions
4. pkg/ui/workspaces/db-console/.gitignore - Removed distoss/distoss.go exclusion
Documentation:
5. pkg/ui/ui.go - Updated package comment to remove "OSS UI" reference
6. pkg/ui/README.md - Renamed "CCL Build" section to "Module Resolution" and updated text
Variable Renaming:
7. pkg/cmd/dev/ui.go - Renamed protoOss → protoBase and ossDst → baseDst
Makefile:
8. GNUmakefile - Removed buildoss from the comment alias example
Verification
Not Changed (preserved intentionally)
Epic: None
Release note: None
Depends on 4 prior PRs for other build upgrades and cleanups: #158877, #158878, #158887, #158900