Skip to content
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

fix: Remove fallback middleware also after Vite restart #18455

Merged
merged 7 commits into from
Jan 22, 2024

Conversation

Artur-
Copy link
Member

@Artur- Artur- commented Jan 15, 2024

The existing hacky way of removing the middleware has two issues:

  1. It removes it AFTER the first request to a non-existing file so the first request for a non-existing resources index.html and subsequent requests return 404

  2. It does not work with Vite 4.0.5+ after restart, see Vite uses and passes a different server instance to plugins after server restart vitejs/vite#15589

The existing hacky way of removing the middleware has two issues:

1. It removes it AFTER the first request to a non-existing file so the first request for a non-existing resources index.html and subsequent requests return 404

2. It does not work with Vite 4.0.5+ after restart, see vitejs/vite#15589
Copy link

github-actions bot commented Jan 15, 2024

Test Results

1 056 files  ± 0  1 056 suites  ±0   1h 20m 14s ⏱️ +38s
6 800 tests ± 0  6 754 ✅ ± 0  46 💤 ±0  0 ❌ ±0 
7 108 runs  +20  7 051 ✅ +20  57 💤 ±0  0 ❌ ±0 

Results for commit 1b933e9. ± Comparison against base commit b00aa55.

♻️ This comment has been updated with latest results.

@mshabarov mshabarov self-requested a review January 15, 2024 12:34
@Artur-
Copy link
Member Author

Artur- commented Jan 16, 2024

You can test the problem using

npx @hilla/cli init hello
mvn
curl http://localhost:8080/VAADIN/static/push/vaadinPush.js # Returns vaadinPush.js
touch vite.config.ts
curl http://localhost:8080/VAADIN/static/push/vaadinPush.js # Returns index.html

Copy link

Quality Gate Passed Quality Gate passed

Kudos, no new issues were introduced!

0 New issues
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarCloud

@mshabarov mshabarov merged commit ae7e360 into main Jan 22, 2024
26 checks passed
@mshabarov mshabarov deleted the remove-middleware-in-a-better-way branch January 22, 2024 14:56
vaadin-bot pushed a commit that referenced this pull request Jan 22, 2024
The existing hacky way of removing the middleware has two issues:

1. It removes it AFTER the first request to a non-existing file so the first request for a non-existing resources index.html and subsequent requests return 404

2. It does not work with Vite 4.0.5+ after restart, see vitejs/vite#15589

Co-authored-by: Peter Czuczor <61667986+czp13@users.noreply.github.com>
vaadin-bot added a commit that referenced this pull request Jan 22, 2024
)

The existing hacky way of removing the middleware has two issues:

1. It removes it AFTER the first request to a non-existing file so the first request for a non-existing resources index.html and subsequent requests return 404

2. It does not work with Vite 4.0.5+ after restart, see vitejs/vite#15589

Co-authored-by: Artur <artur@vaadin.com>
Co-authored-by: Peter Czuczor <61667986+czp13@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants