search

ampproject / amp.dev

The AMP Project Website.
https://amp.dev
Other
584 stars 694 forks source link

<amp-script> sample: why fetch requires an absolute URL #3643

Open morsssss opened 4 years ago

morsssss commented 4 years ago

3588 🐛 Documentation question

This isn't really a bug report, but something in the <amp-script> sample which I've not heard before - and I'm wondering whether it's correct.

The text reads:

"As amp-script code is executed inside a web worker, fetch only works with absolute URLs."

Is that the reason why absolute URLs are required? I haven't seen anything about this with fetch in Workers. It could still be the case. Otherwise, I imagine AMP requires absolute URLs for the same reason it always does: what if your page is served from a cache?

@sebastianbenz , hoping you can answer this one!

sebastianbenz commented 4 years ago

This might be an issue only with inline amp-script. Can you investigate?

morsssss commented 4 years ago

I don't understand... inline amp-script wouldn't specify a URL... can you explain what you mean?

patrickkettner commented 4 years ago

<amp-script> is not running in the same context as the main page. it is run inside of a blob, so a relative path isn't meaningful to it

morsssss commented 4 years ago

That... is a good point.

In which case, this documentation is correct exactly as it is 😎

patrickkettner commented 4 years ago

its is a bummer for DX, is there any way to build some tooling to transform relative URLs to absolute ones? Could make developing on local easier

patrickkettner commented 4 years ago

@morsssss what do you think is the best path forward on this one?

morsssss commented 4 years ago

Thanks for bringing this up again! I'd forgotten your explanation. I've actually just changed our docs to say this happens because AMP always requires absolute URLs and HTTPS.

When I develop locally, I just ignore the warnings. Of course this could be a problem for people who run the validator as part of their pre-commit hooks.

I imagine this problem is identical for anyone using URLs anywhere in AMP - like also in <amp-list>.

That said, looking at our docs for <amp-list>, I see there is actually not a requirement there for an absolute URL. And our sample uses a relative URL.

What do you suggest? I feel like you might have a better idea here than I would....

patrickkettner commented 4 years ago

sortof seems like this should be documented for workerdom, and we link out to it. But if we want it to be self contained, easy path would probably be adding a line about it being inside of a blob and further than to explain why that requires absolute urls. I think an amp optimizer transform would be a great addition, but I am not sure if it is worth the effort (cc @sebastianbenz, if he wanted to chime in)