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

The classic script's source text isn't defined in HTML any more #1212

Closed
jungkees opened this issue Oct 20, 2017 · 4 comments
Closed

The classic script's source text isn't defined in HTML any more #1212

jungkees opened this issue Oct 20, 2017 · 4 comments
Milestone

Comments

@jungkees
Copy link
Collaborator

Update algorithm step 8 compares the updating script's source text against the installed script's source text. This check is to return early when the two scripts are byte identical.

I noticed HTML removed the definition of the source text from the classic script struct and added a record (Script Record) to the script struct instead.

I wonder if comparing the record.[[ECMAScriptCode]] of the two scripts would make sense or we should add the source text to the script struct again. From #1023 (comment), the latter seems to be more appropriate I suppose.

/cc @domenic

@jungkees jungkees added this to the Version 1 milestone Oct 20, 2017
@domenic
Copy link
Contributor

domenic commented Oct 25, 2017

Oh, sorry about this. It does sound like we need to add the source text back to the script record. I will try to add it back soon, per the comment you cited.

@jungkees
Copy link
Collaborator Author

That sounds good. I presume you meant to add it back to the script struct, right? We need non-parsed raw script texts for both classic scripts and module scripts.

@domenic
Copy link
Contributor

domenic commented Oct 26, 2017

Yep.

jungkees added a commit that referenced this issue Mar 5, 2018
This change includes/considers the following:
 - Include imported scripts to byte-check (for classic scripts).
 - Compare responses' body instead of source text as per whatwg/html#3316.
 - Handle duplicate importScripts() as per #1041.
 - Replace *imported scripts updated flag* referenced in importScripts()
   by using service worker's state item.
 - Have Update's perform the fetch steps cover module scripts.
 - Avoid dobule-download of imported scripts pointed out in #1023 (comment).

This change basically makes it check out if the main script resource is
identical to the existing resource. If so, it returns; otherwise, it
creates a new service worker and evalute it to check out if any imported
scripts are changed. It continues with Install only when any of the
resources has been changed. With the change, importScripts() returns
resources from the cache for any duplicated requests including the
request for the main script.

Fixes #1041, #1212, #1023.
jungkees added a commit that referenced this issue Mar 13, 2018
This change includes/considers the following:

 - Include imported scripts to byte-check (for classic scripts).
 - Compare responses' body instead of source text as per whatwg/html#3316.
 - Handle duplicate importScripts() as per #1041.
 - Remove imported scripts updated flag referenced in importScripts() by
   using service worker's state item instead.
 - Have Update's perform the fetch steps cover module scripts.
 - Confirm dobule-download of imported scripts pointed out in #1023 (comment)
   never happens; importing a script again always gets the result from
   the cache.

This change basically gets the imported scripts included to the
byte-check for updates. Update continues with Install if any of the
(main or imported) resources has been changed. This change also makes
any duplicated requests (including the main script) from importScripts()
return the result from the cache.

Fixes #839, #1041, #1212, #1023.
@jungkees
Copy link
Collaborator Author

8e25c26 closes this issue by comparing response bodies instead of source texts.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants