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

Adding self.serviceWorker to service workers. #1426

Merged
merged 2 commits into from
Jun 21, 2019
Merged

Adding self.serviceWorker to service workers. #1426

merged 2 commits into from
Jun 21, 2019

Conversation

jakearchibald
Copy link
Contributor

@jakearchibald jakearchibald commented Jun 7, 2019

Copy link
Member

@mfalken mfalken left a comment

Choose a reason for hiding this comment

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

This looks great... if it's not too much of a hassle, it might also be worth splitting this into two PRs: one with the "getting the service worker object" algorithm and one that adds self.serviceWorker. For self.serviceWorker, it might be worth pinging the issue or here for more visibility?

docs/index.bs Outdated Show resolved Hide resolved
@jakearchibald
Copy link
Contributor Author

if it's not too much of a hassle, it might also be worth splitting this into two PRs: one with the "getting the service worker object" algorithm and one that adds self.serviceWorker

Good point. I'll split it up.

@jakearchibald
Copy link
Contributor Author

jakearchibald commented Jun 7, 2019

Ok, this is only about self.serviceWorker now. It'll fail to build properly because "getting the service worker object" isn't defined. (update: the other PR merged, so the references work)

@jakearchibald
Copy link
Contributor Author

Assuming we want self.serviceWorker to always be available, during the very first run of a service worker it won't yet be in the 'installing' state. Looks like we're going to expose the 'parsed' state for the first time.

@jakearchibald
Copy link
Contributor Author

Tests web-platform-tests/wpt#17285

@mfalken
Copy link
Member

mfalken commented Jun 13, 2019

Seems OK to expose "parsed". I'm a bit curious now how that name was chosen. Is it better than something like "initial"? Really naive question but is JavaScript specified to do parsing before starting execution?

@jakearchibald
Copy link
Contributor Author

Seems OK to expose "parsed". I'm a bit curious now how that name was chosen. Is it better than something like "initial"?

I never thought about changing the name. "initial" seems ok. It could also be "pending", "pendingInstall", "beforeInstall", "preinstall".

Really naive question but is JavaScript specified to do parsing before starting execution?

I think there are cases where v8 defers parsing parts of some functions, but the top level has been parsed at this point.

@wanderview
Copy link
Member

Right, if there was a syntax error you would not have gotten to the point of executing the script.

@jakearchibald
Copy link
Contributor Author

I think I'm favouring "preinstall". Anyone hating that?

@jakearchibald
Copy link
Contributor Author

Let's stick with "parsed". It's past tense like "installed" & "activated".

This pull request was closed.
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.

Find a way for service workers to get their current state
3 participants