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

Index.html in the pathname should be optional #57

Merged
merged 1 commit into from
Mar 12, 2025

Conversation

kmiller68
Copy link
Contributor

When fixing paths in the url for Dart-flute-wasm I neglected to handle the case where index.html is not in the URL. It should now be robust against any rename of index.html too.

When fixing paths in the url for Dart-flute-wasm I neglected to handle the case
where index.html is not in the URL. It should now be robust against any rename
of index.html too.
Copy link

netlify bot commented Mar 12, 2025

Deploy Preview for webkit-jetstream-preview ready!

Name Link
🔨 Latest commit 06c08d4
🔍 Latest deploy log https://app.netlify.com/sites/webkit-jetstream-preview/deploys/67d1e9dc599f4c0008068d50
😎 Deploy Preview https://deploy-preview-57--webkit-jetstream-preview.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@kmiller68
Copy link
Contributor Author

This seems to have fixed the netify preview issue reported in #54. I tested by loading https://deploy-preview-57--webkit-jetstream-preview.netlify.app/ and running it. @mstange do you want to confirm it works for you too?

@@ -269,7 +269,7 @@ class Benchmark {
if (isInBrowser) {
// In browsers, relative imports don't work since we are not in a module.
// (`import.meta.url` is not defined.)
let pathname = location.pathname.match(/(.*)index\.html/)[1];
const pathname = location.pathname.match(/^(.*\/)(?:[^.]+(?:\.(?:[^\/]+))+)?$/)[1];
Copy link

Choose a reason for hiding this comment

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

It might be easier to read if you use replace, for example .replace(/\/index\.html$/, '/')

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, although I'd rather do that in a follow up since I'd like to unblock this.

@mstange
Copy link

mstange commented Mar 12, 2025

@mstange do you want to confirm it works for you too?

I can confirm that https://deploy-preview-57--webkit-jetstream-preview.netlify.app/ completes successfully, thanks!

@kmiller68 kmiller68 merged commit 901c95b into WebKit:main Mar 12, 2025
7 of 10 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.

2 participants