Skip to content

Conversation

SkyeYoung
Copy link
Member

@SkyeYoung SkyeYoung commented Aug 15, 2025

Please answer these questions before submitting a pull request, or your PR will get closed.

Why submit this pull request?

  • Bugfix
  • New feature provided
  • Improve performance
  • Backport patches

What changes will this PR take into?

Note

Note that after merging this PR, we need to test and adjust the corresponding configurations in APISIX.

Considering that #3169 will introduce the larger monaco editor, vite-plugin-compression2 is added to generate smaller gz and br files during the build process, for use when integrating with APISIX. This will help minimize the size as much as possible.

use du -sh dist to compare:

OLD NEW
3.6M dist 1.3M dist

Related issues

fix/resolve #

Checklist:

  • Did you explain what problem does this PR solve? Or what new features have been added?
  • Have you added corresponding test cases?
  • Have you modified the corresponding document?
  • Is this PR backward compatible? If it is not backward compatible, please discuss on the mailing list first

@SkyeYoung SkyeYoung changed the title feat: generate gzip and brotli compressed assets during build feat: generate gzip assets during build Aug 17, 2025
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 adds gzip compression to the build process to reduce bundle size from 3.6M to 1.3M by integrating vite-plugin-compression2. The change prepares the application for better performance when served through APISIX by generating pre-compressed assets.

  • Adds vite-plugin-compression2 dependency to generate gzip assets during build
  • Configures the plugin to create gzip files and delete original assets
  • Updates nginx configuration to serve pre-compressed files with gzip_static

Reviewed Changes

Copilot reviewed 3 out of 4 changed files in this pull request and generated 2 comments.

File Description
vite.config.ts Adds compression plugin configuration to generate gzip assets during build
package.json Adds vite-plugin-compression2 dependency
e2e/server/nginx.conf Updates nginx config to serve pre-compressed gzip files
Files not reviewed (1)
  • pnpm-lock.yaml: Language not supported

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

index index.html;
try_files $uri $uri/ /ui/index.html;
gzip_static always;
error_page 404 /ui/index.html;
Copy link
Preview

Copilot AI Aug 17, 2025

Choose a reason for hiding this comment

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

The removal of try_files $uri $uri/ /ui/index.html; and replacement with error_page 404 /ui/index.html; changes the fallback behavior. The original try_files directive handles SPA routing more comprehensively by trying multiple paths before falling back, while error_page 404 only handles actual 404 errors. This could break client-side routing for the SPA.

Suggested change
error_page 404 /ui/index.html;
try_files $uri $uri/ /ui/index.html;

Copilot uses AI. Check for mistakes.

Copy link
Member Author

Choose a reason for hiding this comment

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

🤔

Copy link
Member

Choose a reason for hiding this comment

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

Curious why changing this line? I would prefer using try_files, which is also documented in React and Vue document sites 😁

Copy link
Member Author

Choose a reason for hiding this comment

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

Curious why changing this line? I would prefer using try_files, which is also documented in React and Vue document sites 😁

Because try_files doesn't work as expected. And this is actually a dev server. There are other configurations in APISIX.

Copy link
Member

Choose a reason for hiding this comment

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

Waiting for other reviewers

Copy link
Contributor

Choose a reason for hiding this comment

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

I tend to use the same configuration as in APISIX in the e2e environment.

Copy link
Member Author

Choose a reason for hiding this comment

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

Currently, the dev server and APISIX work differently.

Copy link
Contributor

Choose a reason for hiding this comment

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

If error_page meets our needs, we can use it, even in APISIX. Just add some comments.

Configuring try_files requires nginx expert, while error_page seems more straightforward.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes. I think if there's a problem, it might be a lack of handling for certain status codes.

}

location /ui/ {
alias /usr/share/nginx/html/;
index index.html;
try_files $uri $uri/ /ui/index.html;
gzip_static always;
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we still need the gunzip module to handle exceptions for clients that don't support gzip?

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm just curious what clients don't support gzip these days...

Copy link
Member Author

@SkyeYoung SkyeYoung Aug 22, 2025

Choose a reason for hiding this comment

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

@SkyeYoung SkyeYoung requested a review from bzp2010 August 27, 2025 01:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants