Deprecate Astro.resolve

[matthewp]

In order to improve build performance we want to prevent scanning rendered HTML in order to discover what assets are used inside of a user's src/ folders. In order to achieve this we need for usage to be static.

Given that assets can be added using an import specifier like:

---
import penguinUrl from '../images/penguin.png?url';
---

<img src={penguinUrl} />

With #23 there is a proposal for sugar around the above. If that proposal goes through we should deprecate Astro.resolve.

Rough proposal

1. Add good documentation to docs.astro.build that provides ways to reference assets within src/ using import specifiers and/or [Draft] Relative path handling in Astro #23 if that goes through.
2. Add a warning on usage of Astro.resolve in a major version.
3. Remove usage of Astro.resolve in 1.0.

[jonathantneal]

Noting that query params are a hack and a solution using import assertions would be preferred here:

import penguinUrl from '../images/penguin.png' assert { type: 'url' };

I think this is significant and warrants our consideration. Intellisense is presently unkind to the query params hack. Assertions are supported in TS now (and AFAIK in our existing language syntax, compiler, and the acorn (?) lexer we use).

Maybe all of this doesn’t mean the query params hack is a show stopper, but once v1.0 ships would we turn around and deprecate it a week or a few months later once it’s settled in Vite? Or would want to have multiple ways to do this for a long time? I think it’s worth seriously considering some pros and cons.

Finally, in a potential catch 22, the last response from Vite was kinda like permission for us to move this along in a PR. ^link

[matthewp]

I don't think I agree that query params are a hack. Query params are a normal way to add additional information about a request. I've interacted with plenty of APIs that used query params to set the type you want to be returned (such as ?json).

I'd argue that import assertions are a hack for this use case, as they are supposed to be for asserting something about what is being imported. As far as I can tell, in a browser/server context, there's no way for the server to know what the type assertion even is, so you couldn't differentiate between the type to send a different response. I don't know what Node has planned for this. See this demo: https://codesandbox.io/s/kind-sun-ygbnv?file=/src/index.js

I agree that import assertions are more aesthetically pleasing than query params, and assuming that we'll be able to implement them in Vite it's no worse than ?url (as far as the query param being a Vite feature as well).

[jonathantneal]

Huh. You’ve convinced me of most of this. My mind is changed and I retract a majority of my statement.

Query params are a normal way to add additional information about a request.

I understand this argument, and I concur. As someone put it "Just pretend transformer plugins are servers".

I realize now that what I consider a 'hack' is just a strongly held opinion™ that I wouldn’t expect a server to transform the response of a URL to returns its own URL. Something about it feels redundant to me.

import penguinUrl from '../images/penguin.png?url';

1. JavaScript internally resolves ../images/penguin.png?url to make the request to the server; then,
2. The server interprets ?url and returns the URL it was given, sans the query parameter.

With that in mind; some questions (if they are in scope) —

Is this another currently valid way to replicate Astro.resolve after its removal?

const penguinUrl = new URL('../images/penguin.png', import.meta.url);

And, hypothetically, if we allowed some a syntactic-sugar scheme like local:, would this be another valid way to replace Astro.resolve?

const penguinUrl = new URL('local:../images/penguin.png');

[matthewp]

The reason to deprecate Astro.resolve is to prevent dynamically constructing URLs to items in src/, since this prevents us from knowing about them during the build (once we remove HTML scanning, that is) and correctly building them out to dist/. Your local:src="../images/penguin.png" idea would presumably be the sugar solution.

[matthewp]

A big downside here is that we have spent the last several months heavily encouraging the use of Astro.resolve for adding any types of assets. I'm suspecting that there are a lot of projects that use it heavily. We need a good migration strategy for them.

Would it make sense to have a version in which Astro.resolve is deprecated and removed as a runtime resolver, but still allow the compiler to turn usage into static imports? And then message that way?

[FredKSchott]

(naively) could we track calls to Astro.resolve() at build-time, collect the result of every call, and then make sure those assets are copied into the final build as-is (or something like that)? This wouldn't work with SSR, but I guess thats one of the perks of not shipping true SSR until post-v1.0.

I would bet (maybe hope) that this would let us keep Astro.resolve() deprecated but not removed for a significant migration period.

[matthewp]

Yes, I think this is a good strategy.