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: fix getCurrentScriptSource to handle scripts without src #331

Merged
merged 2 commits into from
Mar 23, 2021

Conversation

tinytinyeye
Copy link
Contributor

The current implementation of getCurrentScriptSource will fail when there are scripts without src used in the document and the script that runs React app is not the last script in the document.

For example, in the case when there are other 3rd party script snippets such as Qualtrics used in the document, the last script get by the original implementation could be such script and the src will be null. As a result, getCurrentScriptSource will return null and getSocketUrlParts will fail at url.parse(scriptSource || '').

To fix this issue, we could return the last script that HAS src attribute as the current script.

@nticaric
Copy link
Contributor

If the changes in #323 go through this will be automatically fixed

sockets/utils/getCurrentScriptSource.js Outdated Show resolved Hide resolved
sockets/utils/getCurrentScriptSource.js Outdated Show resolved Hide resolved
@tinytinyeye
Copy link
Contributor Author

Hey @pmmmwh, recently I updated my commits to include the changes you suggested and was wondering if you still have other concerns regarding my fix. Thank you!

@pmmmwh pmmmwh merged commit 2689dd7 into pmmmwh:main Mar 23, 2021
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

3 participants