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

Fetching scripts with an invalid type/language attributes is going to be removed in Chrome 56 #2132

Closed
jrx-jsj opened this issue Dec 2, 2016 · 23 comments
Labels

Comments

@jrx-jsj
Copy link

@jrx-jsj jrx-jsj commented Dec 2, 2016

Help us to manage our issues by answering the following:

  1. Describe your issue:

When using scripts for importing uncompiled RIOT tags in a HTML page, we are getting the following notice in Chrome 55:

Fetching scripts with an invalid type/language attributes is deprecated and will be removed in M56, around January 2017. See https://www.chromestatus.com/features/5760718284521472 for more details.

  1. Can you reproduce the issue?

Every webpage using uncompiled RIOT tags will trigger the notice. You can check it in Timer example

  1. On which browser/OS does the issue appear?

Notice triggered on Chrome 55. Feature is planned to be removed on Chrome 56.

  1. Which version of Riot does it affect?

Latest.

  1. How would you tag this issue?
  • Question
  • Bug
  • Discussion
  • Feature request
  • Tip
  • Enhancement
  • Performance
@cognitom cognitom added the discussion label Dec 2, 2016
@cognitom
Copy link
Member

@cognitom cognitom commented Dec 2, 2016

Hi @jrx-jsj, thanks for reporting. As my understanding, the purpose of this change is to prevent "double downloads".

  1. prefetching by browser (deprecate in Chrome 56)
  2. ajax fetching by Riot

So, we'll get no effect on it, I think. If you find any other side effects, let us know.

@GianlucaGuarini
Copy link
Member

@GianlucaGuarini GianlucaGuarini commented Dec 3, 2016

Riot uses ajax requests to load script tags so it should not be affected by the chrome change @jrx-jsj thanks anyway

@zcorpan
Copy link

@zcorpan zcorpan commented Dec 7, 2016

In #2140 I suggest that this should be changed anyway, even though nothing will break, because:

  • currently it is issuing two requests, which is bad for performance.
  • Using the src attribute with an unsupported type is invalid HTML.

See that issue for a proposed fix.

@cognitom
Copy link
Member

@cognitom cognitom commented Dec 7, 2016

@zcorpan thanks.
I saw that CoffeeScript uses data-src, even though I didn't see in others.
http://coffeescript.org/#1.9.2

In the point of readability and analogy to normal script, I think we shouldn't move to data-src. We can just add data-src as an alias to src. Thoughts? @GianlucaGuarini

@zcorpan
Copy link

@zcorpan zcorpan commented Dec 7, 2016

Alternatively you could use something other than script altogether. I just thought it would be the simplest change. But you just need to grab a string from somewhere and have it be convenient to author it in the HTML, right? You could for example use a custom element:

<riot-tag src="dbmon.tag"></riot-tag>

Edit: (however this can't be used in head)

@cognitom
Copy link
Member

@cognitom cognitom commented Dec 7, 2016

In-browser compilation is actually a kind of tools for the beginners. So data-src, or <riot-tag> could be an overload for them to understand. I'm worried about it...

Anyway here's a quick PR #2141.
How about just adding one line at the end of this section in our documentation?

Note: you can also use data-src attribute to prevent double fetching.

@cognitom
Copy link
Member

@cognitom cognitom commented Dec 7, 2016

I'd like to add a link to an external page which explains double fetching.
@zcorpan pls let me know, if you know any good resource about it.

@csharrison
Copy link

@csharrison csharrison commented Dec 7, 2016

Hi cognitom, the chromestatus entry should be the go-to place:
https://www.chromestatus.com/feature/5760718284521472

Unfortunately it has nothing about double fetching it, but I can add a sentence to the description.

@GianlucaGuarini
Copy link
Member

@GianlucaGuarini GianlucaGuarini commented Dec 7, 2016

I still don't get it... we parse the DOM to find <script> nodes with the attribute type="riot/tag". I don't get why should we care if chrome fetches these scripts or not since we load their content via ajax directly in riot anyway. Will chrome remove all these <script> tags from the DOM?

@csharrison
Copy link

@csharrison csharrison commented Dec 7, 2016

Today, Chrome will speculatively look ahead of the HTML parser and preload any script it finds, regardless of type. This means Chrome will download those scripts with type "riot/tag" twice: once during the preload stage and once when the library actually initiates the XHR.

We are tightening the restrictions here so we only speculatively fetch resources with valid types (i.e. ones that will actually execute), which will stop the double download in this case.

@rsbondi
Copy link
Contributor

@rsbondi rsbondi commented Dec 7, 2016

This sounds like a good thing.

@csharrison
Copy link

@csharrison csharrison commented Dec 7, 2016

It is a good thing, but we haven't shipped it yet :)

We are aiming to remove the fetch in Chrome 56. If riot moves away from this model today, the double downloading will stop in Chrome 55, along with Safari, Firefox, etc.

@GianlucaGuarini
Copy link
Member

@GianlucaGuarini GianlucaGuarini commented Dec 7, 2016

Ok so this only means that all the script tags will not be fetched by Chrome automatically but only by riot when it will be needed by calling riot.compile so that's why I guess we don't need to change anything. This change does not affect riot it just fixes a wrong chrome behavior. Do we really need also data-src at this point?

@cognitom
Copy link
Member

@cognitom cognitom commented Dec 7, 2016

I thought so, too. They said "Deprecate all fetches with invalid type/language".
Is this correct?

  • now: prefetch + fetch via ajax
  • future: prefetch(deprecate) fetch via ajax
@csharrison
Copy link

@csharrison csharrison commented Dec 7, 2016

As I mentioned in my previous comment, many other browsers still make this extra fetch, including the current version of Chrome. zcorpan also mentioned this in #2132 (comment).

@cognitom
Copy link
Member

@cognitom cognitom commented Dec 7, 2016

Ah, I see. I've confirmed "double fetching" in Firefox, too.

image

@GianlucaGuarini
Copy link
Member

@GianlucaGuarini GianlucaGuarini commented Dec 7, 2016

Ok so we don't need to change anything..I would like to leave the src attribute, and if there will be a prefetch, the ajax request will just use the cache to retrieve the script contents

@cognitom
Copy link
Member

@cognitom cognitom commented Dec 7, 2016

@GianlucaGuarini just curious, why Firefox says 200 at the second fetch?

@rsbondi
Copy link
Contributor

@rsbondi rsbondi commented Dec 7, 2016

In edge and chrome I see a double request, but the second request returns status 304, so appears to be only a single fetch as confirmed by the timeline, sounds like the new chrome behavior is what makes the most sense

@cognitom
Copy link
Member

@cognitom cognitom commented Dec 7, 2016

Gotcha, the case above is just because of overlapping.

image

@GianlucaGuarini
Copy link
Member

@GianlucaGuarini GianlucaGuarini commented Dec 7, 2016

@cognitom http://plnkr.co/edit/l5nzSwQN01gyE01DoCei?p=preview check this example. The file gets retrieved from the cache if the riot.compile gets called a bit later

@cognitom
Copy link
Member

@cognitom cognitom commented Dec 7, 2016

@GianlucaGuarini it could depend on the latency and the size of the page.

Anyway I have no preference to support data-src or not. It could be worth for pros in some rare case, but pros might not use in-browser compiler. On the other hand beginners may not care about it.

GianlucaGuarini added a commit that referenced this issue Dec 7, 2016
Closes #2132: data-src attr for script tag
@zcorpan
Copy link

@zcorpan zcorpan commented Dec 7, 2016

The prefetch and the XHR fetch can't use the same connection (if the XHR starts before the prefetch is finished), because a script prefetch and XHR fetch have different rules/semantics in the engine. That's why there is a double request.

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

Successfully merging a pull request may close this issue.

None yet
6 participants
You can’t perform that action at this time.