Allow a cookies to be included with cross-origin calls for icons

[rpemberton]

I'm having problems making cross-origin calls for icons (I don't see how to include a cookie with those).

One solution may be to provide a way to add credentials: "include" to the request options here

Related resources:
https://developer.mozilla.org/en-US/docs/Web/API/Request/credentials#include

[claviska]

This would require some kind of higher-level configuration outside the component. Have you considered enabling CORS on the target server instead? This is how we handle it for all the icon libraries via public CDN.

[rpemberton]

Have you considered enabling CORS on the target server instead?

I think the issue is that the target server will simply reject any requests that don't include a valid cookie. And it'll do that regardless of CORS settings and the request origin.

This would require some kind of higher-level configuration outside the component.

This would be much appreciated! I'm using a modified fork of Shoelace at the moment as I don't see any other way to include a cookie with these requests.

[claviska]

I'm not sure this is something we want to support since it can be solved with a CORS config and since all of our existing use cases are already covered.

I'm assuming your icon assets are on a system that requires auth, hence the cookie. I'm not sure how many folks out there will need something like this. Since you're already forking, have you considered prototyping this change so we can see what you have in mind?

[rpemberton]

I think I may have misunderstood the original CORS config suggestion. Were you suggesting dropping the cookie requirement because CORS config makes it unnecessary?

Something I noticed is that when I use an img element to make the same cross-origin requests for icons, the cookie is automatically included in the request. For example a website https://foo.com with <img src="https://assets.foo.com/chevron-down.svg" />. I wonder if sl-icon should share this behavior?

My fork is very simple. It directly adds credentials: 'include' to the fetch options object. Making it configurable may be more appropriate though. If including credentials is something you'd like to support, I may be able to find time to contribute a PR.

[rpemberton]

Maybe this could be configured at library level, either as a boolean or as a function that returns a boolean:

diff --git a/src/components/icon/library.ts b/src/components/icon/library.ts
index 202f54ac..7c2ec9df 100644
--- a/src/components/icon/library.ts
+++ b/src/components/icon/library.ts
@@ -9,6 +9,7 @@ export interface IconLibrary {
   resolver: IconLibraryResolver;
   mutator?: IconLibraryMutator;
   spriteSheet?: boolean;
+  includeCredentials?: (url: string) => boolean;
 }
 
 let registry: IconLibrary[] = [defaultLibrary, systemLibrary];