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: update client.ts@cleanUrl to accomodate blob protocol #16182

Merged
merged 3 commits into from
Mar 19, 2024

Conversation

ferdisap
Copy link
Contributor

Description

I would like to display view in browser using blob. But it is not 'hot update' because the using of blob protocol on the url blob:http://localhost:8000/13a653f9-99c8-472c-94a2-7ed6b5c9b917. It cause an TypeError: Failed to construct 'URL': Invalid URL at line client.ts:126.
Capture
Before: const url = new URL(pathname, location.toString());
After: const url = new URL(pathname, location.protocol === 'blob:' ? location.pathname : location.toString());

Additional context


What is the purpose of this pull request?

  • Bug fix
  • New Feature
  • Documentation update
  • Other

Before submitting the PR, please make sure you do the following

  • Read the Contributing Guidelines, especially the Pull Request Guidelines.
  • Check that there isn't already a PR that solves the problem the same way to avoid creating a duplicate.
  • Provide a description in this PR that addresses what the PR is solving, or reference the issue that it solves (e.g. fixes #123).
  • Update the corresponding documentation if needed.
  • Ideally, include relevant tests that fail without this PR but pass with it.

i would like to display view in browser using blob. But it is not 'hot update' because the using protocol 'blob'. It cause an TypeError: Failed to construct 'URL': Invalid URL at client.ts@cleanUrl. So thats why i change the code at client.ts@cleanUrl, line 114.
Copy link

stackblitz bot commented Mar 17, 2024

Review PR in StackBlitz Codeflow Run & review this pull request in StackBlitz Codeflow.

@ferdisap ferdisap changed the title Update client.ts@cleanUrl to accomodate blob protocol Fix: Update client.ts@cleanUrl to accomodate blob protocol Mar 17, 2024
@ferdisap ferdisap changed the title Fix: Update client.ts@cleanUrl to accomodate blob protocol fix: Update client.ts@cleanUrl to accomodate blob protocol Mar 17, 2024
@ferdisap ferdisap changed the title fix: Update client.ts@cleanUrl to accomodate blob protocol fix: update client.ts@cleanUrl to accomodate blob protocol Mar 17, 2024
@sapphi-red
Copy link
Member

Would you create a reproduction or add a test? I don't understand when this happens.

@ferdisap
Copy link
Contributor Author

ferdisap commented Mar 18, 2024

Would you create a reproduction or add a test? I don't understand when this happens.

I tried to make this dummy.

TESTING VITE using Blob Source in <Iframe>

  1. Start php server
  2. Open second terminal, run npm run dev
  3. Hit /main.php (i suggest to open in new tab to see the network, source, and console). Click to create and show blob url in iframe. You will see the thext 'This is my frame example' in iframe.
  4. Back to editor and try to edit content of myFrame.css. In console page, you will see an TypeError: Failed to construct 'URL': Invalid URL at client.ts:114
  5. If we see in the page network list, websocket has been ran well. If we see in npm terminal, vite has been detect the changed file (myFrame.css). I try to change the client.ts:114 in my local computer and it solved.

Please check this out

@sapphi-red
Copy link
Member

I see. I'm not sure if we would support that case but I'm fine with merging this PR as the change is small.

A reproduction without PHP: https://stackblitz.com/edit/vitejs-vite-sug7dr?file=main.js&terminal=dev

@sapphi-red sapphi-red added p2-edge-case Bug, but has workaround or limited in scope (priority) feat: hmr labels Mar 18, 2024
@ferdisap ferdisap requested a review from sapphi-red March 18, 2024 16:46
@patak-dev patak-dev merged commit 1a3b1d7 into vitejs:main Mar 19, 2024
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feat: hmr p2-edge-case Bug, but has workaround or limited in scope (priority)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants