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

Endless string of "undefined" prints after original content string #4

Closed
gazj opened this issue Feb 22, 2018 · 8 comments
Closed

Endless string of "undefined" prints after original content string #4

gazj opened this issue Feb 22, 2018 · 8 comments

Comments

@gazj
Copy link
Contributor

gazj commented Feb 22, 2018

In a statement on line 26 of src/index.js, the finished variable is compared against the value content.length. If they are equivalent, the intervals are cleared and the scifi function execution ends.

However, it seems that the finished variable can occasionally become incremented beyond the value of content.length before that statement is evaluated. When this happens, the content on the page is repeated appended with "undefined".
scifi_undefined

@gazj
Copy link
Contributor Author

gazj commented Feb 23, 2018

Is the demo site updated with the latest changes from pull request #6? If so, I can test for this issue on that page today.

@egoist
Copy link
Owner

egoist commented Feb 23, 2018

@garylocke yes, it should be updated (since it's using the CDN version)

@gazj
Copy link
Contributor Author

gazj commented Feb 23, 2018

Still an issue. I am noticing that I can only reproduce it by running the demo site in a background tab, which leads me to believe it may have something to do with a race condition between the two intervals, though I'm not certain of that.

Pull request #7 introduces a check on the content character before appending it to the finished string. This should eliminate the possibility of finishedString.length ever exceeding content.length, allowing the clearInterval statements to always execute eventually.

@gazj
Copy link
Contributor Author

gazj commented Feb 23, 2018

Once the CDN version is updated, we can try again to reproduce this issue.

@egoist
Copy link
Owner

egoist commented Feb 23, 2018

Yeah updated.

@gazj
Copy link
Contributor Author

gazj commented Feb 23, 2018

Great. I'm testing this in two tabs, one of which is running in the background.

@gazj
Copy link
Contributor Author

gazj commented Feb 23, 2018

Over an hour in the background without any issue. I'm comfortable calling this fixed for now if you are.

EDIT: Now running without issue for several hours.

This was referenced Feb 23, 2018
@gazj
Copy link
Contributor Author

gazj commented Mar 7, 2018

I'm going to close this, since I am no longer able to reproduce the issue.

@gazj gazj closed this as completed Mar 7, 2018
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

No branches or pull requests

2 participants