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

Fixes ScriptPath for script paths with query strings #28

Merged
merged 1 commit into from
Feb 7, 2018

Conversation

shaunol
Copy link
Contributor

@shaunol shaunol commented Feb 7, 2018

Hi there, thank you for publishing such a great project! I've just been getting a demo up and running using Visual Studio 2017 (using standard VS templates). By default, the VS/webpack toolchain will append a version hash query string to its script bundle references. e.g:

http://localhost:59090/dist/main.js?v=hyw4nPT-JXNYaAsGUUqAfkThIaWT7qch0FVyfgwK9zg

The ScriptPath object purposefully throws an exception in order to extract the script path from the stack trace. A regular expression is used to extract the path components. The expression does not expect a query string in the script path.

In my particular case, the stack trace string with the query string on the script path looks like this:

    at Object.module.exports (http://localhost:59090/dist/main.js?v=0gN5LBVFBXS_KkM6Si37YaPTqhTdNsQRWgUcbcGODYs:2884:211)

This fails with error:

Uncaught TypeError: Cannot read property '2' of null
    at ScriptPath.path (perspective.view.js:1)
    at ScriptPath.host (perspective.view.js:1)
    at Object.worker (perspective.view.js:2)
    at get_worker (perspective.view.js:1)
    at HTMLElement.load (perspective.view.js:1)
    at HTMLElement.value (perspective.view.js:1)
    at HTMLDocument.<anonymous> (Home.tsx:24)
    at webcomponents.min.js:14

If we purposefully omit the query string like so:

at Object.module.exports (http://localhost:59090/dist/main.js:2884:211)

It works.

As a fix we can update the regular expression to lazily match any character 0 or more times between the file extension (js|html) and the first instance of the colon character (which in this context, indicates the end of the path).

I applied this fix in my local development environment and had no further issues and also confirmed with a web-based visual regular expression debugger that both forms are correctly parsed.

Thanks!

@texodus
Copy link
Member

texodus commented Feb 7, 2018

Ah, great catch. Verified this works as well - thanks for the fix!

@texodus texodus merged commit 2128990 into finos:master Feb 7, 2018
texodus pushed a commit that referenced this pull request Feb 26, 2019
Fixed css labelling and spacing and some Edge issues
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.

None yet

2 participants