-
Notifications
You must be signed in to change notification settings - Fork 17
feat: migrate to rsbuild #3137
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
feat: migrate to rsbuild #3137
Conversation
c3a9bc6 to
5e4de33
Compare
public/index.html
Outdated
| window.web_version = "<%= !process.env.REACT_APP_BACKEND %>" | ||
| window.custom_backend = "<%= process.env.NODE_ENV === 'development' && process.env.REACT_APP_BACKEND %>" | ||
| window.meta_backend = "<%= process.env.REACT_APP_META_BACKEND %>" | ||
| window.react_app_disable_checks = "<%= process.env.REACT_APP_DISABLE_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.
Changed templates format from % % to "<%= %>. Other changes are deleted comments and applied prettier
|
|
||
| // Mock window.location.origin that is used inside prepareErrorStack | ||
| const windowSpy = jest.spyOn(window, 'window', 'get'); | ||
| windowSpy.mockImplementation(() => { |
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.
Newer version of jest-environment-jsdom does not allow to manipulate window manually. So I had to replace all window mocks with different approach.
This test now expects localhost:3000 base url from config
| @@ -0,0 +1,7 @@ | |||
| module.exports = { | |||
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.
babel config is needed for jest tests to work
| "prettier --write" | ||
| ] | ||
| }, | ||
| "jest": { |
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.
jest config now in its own separate file
5e4de33 to
5fc690b
Compare
| export const webVersion = Boolean(parseJson(window.web_version)); | ||
| export const customBackend = parseJson(window.custom_backend); | ||
| export const metaBackend = parseJson(window.meta_backend); | ||
| export const codeAssistBackend = parseJson(window.code_assist_backend); |
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.
Previously they were parsed automatically and we only faced problem with 'undefined' being a string
3e68add to
87d2115
Compare
87d2115 to
97a8568
Compare
| jobs: | ||
| e2e_tests: | ||
| name: Playwright Tests | ||
| timeout-minutes: 60 |
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.
Now e2e tests can run for a very long time (hours), when they are totally broken, set timeout for such case
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.
17 files reviewed, 1 comment
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 migrates the build system from Create React App (CRA) to Rsbuild, a modern and faster bundler. The migration involves replacing CRA's configuration with Rsbuild's declarative config, updating build tools, and adjusting how environment variables are injected into the application.
Key changes:
- Replaced
react-scriptsandreact-app-rewiredwith Rsbuild and its plugins - Migrated Jest configuration from package.json inline config to dedicated jest.config.ts
- Updated environment variable injection to use rsbuild's templating system in index.html
- Modified dynamic SVG imports to include
?reactquery parameter for proper SVGR processing
Reviewed changes
Copilot reviewed 15 out of 18 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| rsbuild.config.ts | New Rsbuild configuration defining build setup, environment variables, plugins, and Monaco editor integration |
| babel.config.js | New Babel configuration for Jest testing with TypeScript and React support |
| jest.config.ts | Migrated Jest configuration from package.json to dedicated TypeScript config file |
| package.json | Updated scripts to use rsbuild commands, added rsbuild dependencies, removed CRA dependencies |
| public/index.html | Updated environment variable injection from CRA placeholders to rsbuild template expressions |
| tsconfig.json | Changed module resolution options to lowercase (stylistic change, no functional impact) |
| eslint.config.mjs | Updated to reference babel.config.js instead of removed config-overrides.js |
| playwright.config.ts | Added conditional retries for CI environment and clarified testDir path |
| src/types/assets.d.ts | Added type declaration for *.svg?react imports to support rsbuild's SVGR plugin |
| src/utils/utils.ts | Updated parseJson to handle string "undefined" and accept unknown type parameter |
| src/store/configureStore.ts | Added parseJson calls to convert string environment variables to proper types |
| src/components/Illustration/Illustration.tsx | Added ?react suffix to dynamic SVG imports for proper SVGR processing |
| src/store/test/getUrlData.test.ts | Replaced jest.spyOn window mocking with direct window.history.pushState calls |
| src/utils/test/parseBalancer.test.ts | Replaced jest.spyOn window mocking with direct window property assignments |
| src/components/ErrorBoundary/tests/prepareErrorStack.test.ts | Removed window mocking since jsdom provides localhost:3000 by default |
| config-overrides.js | Removed CRA configuration overrides file (no longer needed with Rsbuild) |
Comments suppressed due to low confidence (1)
src/store/test/getUrlData.test.ts:147
- Test pollution risk: Directly mutating
window.historyusingpushStatewithout cleanup can cause test state to leak between tests. The URL changes persist and could affect other tests.
Consider adding cleanup to restore the original state:
describe('getUrlData', () => {
const originalPathname = window.location.pathname;
afterEach(() => {
window.history.pushState({}, '', originalPathname);
});
// ... rest of tests
});describe('getUrlData', () => {
describe('multi-cluster version', () => {
test('should parse pathname with folder', () => {
window.history.pushState(
{},
'',
'/ui/cluster?clusterName=my_cluster&backend=http://my-node:8765',
);
const result = getUrlData({singleClusterMode: false, customBackend: undefined});
expect(result).toEqual({
basename: '/ui',
backend: 'http://my-node:8765',
clusterName: 'my_cluster',
});
});
test('should parse pathname with folder and some prefix', () => {
window.history.pushState(
{},
'',
'/monitoring/ui/cluster?clusterName=my_cluster&backend=http://my-node:8765',
);
const result = getUrlData({singleClusterMode: false, customBackend: undefined});
expect(result).toEqual({
basename: '/monitoring/ui',
backend: 'http://my-node:8765',
clusterName: 'my_cluster',
});
});
test('should parse pathname without folder', () => {
window.history.pushState(
{},
'',
'/cluster?clusterName=my_cluster&backend=http://my-node:8765',
);
const result = getUrlData({singleClusterMode: false, customBackend: undefined});
expect(result).toEqual({
basename: '',
backend: 'http://my-node:8765',
clusterName: 'my_cluster',
});
});
test('should extract environment from first segment', () => {
window.history.pushState(
{},
'',
'/cloud-prod/cluster?clusterName=my_cluster&backend=http://my-node:8765',
);
const result = getUrlData({
singleClusterMode: false,
customBackend: undefined,
allowedEnvironments: ['cloud-prod', 'cloud-preprod'],
});
expect(result).toEqual({
basename: '',
backend: 'http://my-node:8765',
clusterName: 'my_cluster',
environment: 'cloud-prod',
});
});
test('should extract environment from first segment with monitoring folder', () => {
window.history.pushState(
{},
'',
'/cloud-preprod/api/meta3/proxy/cluster/pre-prod_global/monitoring/cluster/tenants',
);
const result = getUrlData({
singleClusterMode: false,
customBackend: undefined,
allowedEnvironments: ['cloud-prod', 'cloud-preprod'],
});
expect(result).toEqual({
basename: '/cloud-preprod/api/meta3/proxy/cluster/pre-prod_global/monitoring',
backend: undefined,
clusterName: undefined,
environment: 'cloud-preprod',
});
});
test('should not extract environment if not in allowed list', () => {
window.history.pushState({}, '', '/cluster/tenants?clusterName=my_cluster');
const result = getUrlData({
singleClusterMode: false,
customBackend: undefined,
allowedEnvironments: ['cloud-prod', 'cloud-preprod'],
});
expect(result).toEqual({
basename: '',
backend: undefined,
clusterName: 'my_cluster',
environment: undefined,
});
});
test('should not extract environment without allowedEnvironments list', () => {
window.history.pushState({}, '', '/my-env/cluster?clusterName=my_cluster');
const result = getUrlData({
singleClusterMode: false,
customBackend: undefined,
});
expect(result).toEqual({
basename: '',
backend: undefined,
clusterName: 'my_cluster',
environment: undefined,
});
});
});
describe('single-cluster version with custom backend', () => {
test('should parse correclty parse pathname', () => {
window.history.pushState({}, '', '/cluster');
const result = getUrlData({
singleClusterMode: true,
customBackend: 'http://my-node:8765',
});
expect(result).toEqual({
basename: '',
backend: 'http://my-node:8765',
});
});
});
describe('single-cluster embedded version', () => {
test('should parse pathname with folder', () => {
window.history.pushState({}, '', '/monitoring/cluster');
const result = getUrlData({singleClusterMode: true, customBackend: undefined});
expect(result).toEqual({
basename: '/monitoring',
backend: '',
});
});
test('should parse pathname with folder and some prefix', () => {
window.history.pushState({}, '', '/node/12/monitoring/cluster');
const result = getUrlData({singleClusterMode: true, customBackend: undefined});
expect(result).toEqual({
basename: '/node/12/monitoring',
backend: '/node/12',
});
});
});
});
You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.
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.
20 files reviewed, 2 comments
package.json
Outdated
| "build:embedded": "GENERATE_SOURCEMAP=false PUBLIC_URL=. REACT_APP_BACKEND=http://localhost:8765 REACT_APP_META_BACKEND=undefined npm run build", | ||
| "build:embedded-mc": "GENERATE_SOURCEMAP=false PUBLIC_URL=. REACT_APP_BACKEND= REACT_APP_META_BACKEND= npm run build", | ||
| "start": "rsbuild dev", | ||
| "dev": "DISABLE_ESLINT_PLUGIN=true TSC_COMPILE_ON_ERROR=true REACT_APP_BACKEND=http://localhost:8765 REACT_APP_META_BACKEND=undefined rsbuild dev", |
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.
it may be left untouched - npm start will invoke correct command
4730221 to
520d14a
Compare
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.
20 files reviewed, no comments
Closes #2236
Stand: https://nda.ya.ru/t/ESJMU9iT7NhbJ2
CI Results
Test Status:⚠️ FLAKY
📊 Full Report
Test Changes Summary ⏭️2
⏭️ Skipped Tests (2)
Bundle Size: 🔽
Current: 62.32 MB | Main: 66.06 MB
Diff: 3.74 MB (-5.65%)
✅ Bundle size decreased.
ℹ️ CI Information