vite icon indicating copy to clipboard operation
vite copied to clipboard

fix(html): public asset urls always being treated as paths (fix #11857)

Open Codex- opened this issue 2 years ago • 3 comments
trafficstars

Description

fix #11857

Adds a simple isUrl utility function that simply uses the node implementation of URL to validate a URL before attempting to normalize the value.

Additional context

Added tests, and I've tested this works against my issue repo with both a URL and non-url path, got the expected results.


What is the purpose of this pull request?

  • [x] Bug fix
  • [ ] New Feature
  • [ ] Documentation update
  • [ ] Other

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

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

Codex- avatar Jan 31 '23 22:01 Codex-

Unsure why the macos tests timeout, I ran the tests successfully locally on macos 🤔

Codex- avatar Jan 31 '23 22:01 Codex-

Thanks for the PR @Codex-! The MacOS test was a flake, don't worry. Would you add a test case for this one? https://github.com/vitejs/vite/blob/main/CONTRIBUTING.md#extending-the-test-suite

patak-dev avatar Feb 01 '23 13:02 patak-dev

@patak-dev really interesting test suite with playground, was interesting to dig into!

I've adapted the relative tests in assets to a new suite url, hopefully this makes sense to do.

I also validated the testing by stepping through with debugging to check we're asserting the path for a URL as expected, and explicitly so double slashes don't get broken accidentally

Codex- avatar Feb 02 '23 00:02 Codex-

  • Rebased
  • Updated added tests to match upstream move to esm

@patak-dev is there anything you want from me to get this moving? :)

Codex- avatar Mar 15 '23 22:03 Codex-