Properly resolve index.html and other entrypoint links #469
Conversation
src/utils.ts
Outdated
| @@ -478,6 +483,18 @@ export function replaceWebRootInSourceMapPathOverridesEntry(webRoot: string, ent | |||
| return entry; | |||
| } | |||
|
|
|||
| export function addEntrypointIfNeeded(sourcePath: string, defaultEntrypoint: string): string { | |||
| try { | |||
There was a problem hiding this comment.
What in this codeblock causes an exception we can't anticipated? It's not clear to me why we need a try/catch if we aren't logging an exception.
There was a problem hiding this comment.
So, the new URL() call will throw if the passed in string isn't a valid url. I can't think of a case where that would be, but I added the try catch just to be safe.
I'd be open to letting it throw throw and then catching in the devtoolsPanel and reporting an error instead of attempting to open the file. I'll go ahead and make that change
There was a problem hiding this comment.
LGTM with one suggestion.
Discussed on a call. Would be good to have an error toast notification to suggest users to change the defaultEntrypoint setting if the source path is modified via the addEntrypointIfNeeded function and it still fails when trying to show the document.
|
Moving this back to draft. I want to rebase this off Vidal's incoming error message handling PR |
test and lint pr feedback fix tests
bgoddar
force-pushed
the
bgoddar/htmlLinkFix
branch
from
edc044e to
a740ae6
Compare
This PR fixes an issue where index.html and other entrypoint links in the extension would not properly resolve to workspace folders. This PR adds a setting to allow users to set the entrypoint of their site, otherwise it defaults to index.html. This causes links like "http://localhost:8080/" from the devtools to now properly resolve to "${workspacefolder}/index.html" It also prompts users to update this setting on cases where default entrypoint does not match a known file.
resolves #420