Mark import as external

[sapphi-red]

This PR adds a way to mark an import as external with resolver.

closes #479
closes #555

[sapphi-red]

For now, I made resolve to accept string | true to focus on the rough design. I think string | { specifier: string, external?: boolean } would be nice here. What do you think?

[devongovett]

yeah or maybe just string | {external: string}?

[sapphi-red]

This case can be bundled without any semantics change, but for the following case, it's difficult to keep the semantics.

@import './b.css'; // bundled
@import "https://fonts.googleapis.com/css2?family=Roboto&display=swap"; // externalized

related: postcss/postcss-import#536, parcel-bundler/parcel#5840 (comment), evanw/esbuild#465 (comment)

Maybe it's fine to simply change the semantics in this case? (both postcss-import and esbuild seems to do that)

[sapphi-red]

I'm not familiar with the rust libraries' semver compat, but I guess changing the return value of a public trait is a breaking change. Is it fine to introduce a breaking change? If not, I'll try to add resolve_advanced method with a default implementation so that it won't be a breaking change.
(On the JS side, it's not a breaking change)

[devongovett]

it's ok, the crate is still under pre-release

[sapphi-red]

@devongovett Would you take this one a look? This is blocking Vite stabilize the lightningcss support.

[devongovett]

As you mentioned, one issue is that it will change the behavior of the CSS if external and internal imports are mixed.

@import "other.css";
@import "http://example.com/external.css";
@import "another.css";

If other.css and another.css get bundled together, but external.css remains as an import the order will change. We could potentially enforce that all external imports must be declared ahead of bundled dependencies maybe. What do you think? Is that too restrictive?

[sapphi-red]

I think it's too restrictive to error on it. I assume it won't matter in most cases. But I think it'd be nice to have a warning.

[devongovett]

Currently the bundler produces invalid css when externals are after bundled imports: other.css gets inlined and the external.css import is placed after it. However CSS requires that @import rules are before all other rules, except @layer declarations so this won't work. If we don't want to emit an error, we will have to hoist the external imports to the top of the bundle.

However when layers are involved this gets complicated.

@layer a;
@import 'a.css';
@layer b;
@import 'external.css';

Here the user has declared layers a and b before external.css. Ideally we would retain this. So somehow we would produce this I think:

@layer a;
@layer b;
@import 'external.css';

/* a.css */

But what if @layer b was actually declared inside a.css instead of inline? Then it gets more complicated and we essentially need to track layers globally across all files and interleave external imports in the right places.

It would be much simpler to emit an error if externals are found after bundled imports. I am tempted to try this and see what issues people run into. We could potentially try implementing that later (or even a simpler approach where all externals are hoisted to the top regardless of layers). But perhaps that should be behind an unsafe flag or something. What do you think?

[sapphi-red]

Starting with that sounds good to me.

Maybe we can tell users to add layer(...)s to external imports that are after bundled imports. If the external imports has a layer function, we can hoist the @import and generate @layer before the @imports based on the correct order.

/* # input */
/* ## index.css */
@import 'a.css'
@import 'external1.css' layer(b);
@import 'c.css'

/* ## a.css */
@layer a;
.a {}

/* ## c.css */
@layer c;
.c {}

/* ---------------- */
/* # output */
@layer a, b;
@import 'external1.css' layer(b);
.a {}
@layer c;
.c {}

Just in case, this example:

@layer a;
@import 'a.css';
@layer b;
@import 'external.css';

is not a valid code as @import must be consecutive. A correct input would be like:

/* ## index.css */
@import 'a.css'
@import 'b.css'

/* ## a.css */
@layer a;
@import 'external1.css';

/* ## b.css */
@layer b;
@import 'external2.css';

which requires layer "b" to come after external1.css but before external2.css. I guess it's impossible to keep the semantics unless we generate multiple files, or use the @import 'data:text/css,...' trick.

[devongovett]

Oh right, I forgot about the consecutive requirement. That makes it slightly simpler.

And good idea about the layers. Unfortunately that affects the behavior with unlayered styles too (e.g. inline styles on the page that we can't see) since unlayered styles always take precedence over layered styles. a.css and b.css would also need to be in layers. Automatically adding layers wouldn't account for other css on the page like inline style elements that we can't see at build time. But perhaps there is something there we can do.

I'd still like to start with an error and see how common of an issue it is. It has a relatively simple fix (moving the external css above the bundled styles), but if that's common in libraries that users can't edit then perhaps we need to add an option.

[sapphi-red]

Thank you for getting this over the line!