Skip to content

👻 Rsbuild migration#365

Merged
carlosthe19916 merged 4 commits intoguacsec:mainfrom
carlosthe19916:rsbuild
Mar 21, 2025
Merged

👻 Rsbuild migration#365
carlosthe19916 merged 4 commits intoguacsec:mainfrom
carlosthe19916:rsbuild

Conversation

@carlosthe19916
Copy link
Copy Markdown
Collaborator

@carlosthe19916 carlosthe19916 commented Feb 22, 2025

@carlosthe19916 carlosthe19916 marked this pull request as draft February 22, 2025 19:41
@carlosthe19916 carlosthe19916 linked an issue Feb 22, 2025 that may be closed by this pull request
Copy link
Copy Markdown
Contributor

@gildub gildub left a comment

Choose a reason for hiding this comment

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

@carlosthe19916, similarly as the Vite solution, it removes 21,000 lines of code and adds about 10,500 lines, even better than vite.

Did you use the migration option to convert from Webpack ?

@carlosthe19916
Copy link
Copy Markdown
Collaborator Author

@carlosthe19916, similarly as the Vite solution, it removes 21,000 lines of code and adds about 10,500 lines, even better than vite.

Did you use the migration option to convert from Webpack ?

No i didn't. I did all changes manually.
Important to notice that there are 2 tools:

Rsbuild contains a lot of "out of the box" configuration similar to Vite, and I did this POC with that.

Signed-off-by: Carlos Feria <2582866+carlosthe19916@users.noreply.github.com>
Signed-off-by: Carlos Feria <2582866+carlosthe19916@users.noreply.github.com>
# Conflicts:
#	client/package.json
#	package-lock.json
#	package.json
@carlosthe19916 carlosthe19916 marked this pull request as ready for review March 14, 2025 14:03
@carlosthe19916 carlosthe19916 changed the title 👻 WIP Rsbuild migration 👻 Rsbuild migration Mar 14, 2025
@carlosthe19916 carlosthe19916 requested a review from gildub March 20, 2025 09:56
@carlosthe19916
Copy link
Copy Markdown
Collaborator Author

@gildub I made minor changes recently to this PR so it is now ready to be reviewed. Whenever you have time, feel free to look at it for the second time :)

export const AnalyticsContextProvider: React.FC<IAnalyticsProviderProps> = ({
children,
}) => {
/* eslint-disable react-hooks/rules-of-hooks */
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.

Biome doesn't use eslint stanza style.
Shouldn't that one be replace with // biome-ignore lint/react-hooks-rules-of-hooks ?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

@gildub I removed the changes this PR was doing in regards of adding comments like /* eslint-disable react-hooks/rules-of-hooks */ As you correctly mentioned it should be replaced by the Biome style.

We will need to do the migration in 2 steps:

  • First step: this PR
  • Second step: enable the biome lint and format rules in our CI and while doing it add the appropriate /* eslint-disable react-hooks/rules-of-hooks */ or similar rules. At the moment we don't enable our Eslint rules in CI either.

Copy link
Copy Markdown
Contributor

@gildub gildub left a comment

Choose a reason for hiding this comment

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

I wonder about the /* eslint-disable ... */ left in the code.
Unless I'm mistaken, don't we need to replace them with /* biome-ignore ... */ equivalent ?

Besides above it looks really good and it's amazingly fast !!!

Signed-off-by: Carlos Feria <2582866+carlosthe19916@users.noreply.github.com>
@carlosthe19916 carlosthe19916 requested a review from gildub March 20, 2025 16:57
Copy link
Copy Markdown
Contributor

@gildub gildub left a comment

Choose a reason for hiding this comment

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

LGTM

@carlosthe19916 carlosthe19916 merged commit 1b3362a into guacsec:main Mar 21, 2025
9 checks passed
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.

Adopt Vite, Rsbuild/Rspack, or similar

2 participants