sergiodxa / remix-utils

A set of utility functions and types to use with Remix.run
https://sergiodxa.github.io/remix-utils/
MIT License
2.06k stars 116 forks source link

`ExternalScriptsHandle` does not attach custom attributes to link #309

Open IgnisDa opened 6 months ago

IgnisDa commented 6 months ago

Describe the bug

Here is my handle function:

export const handle: ExternalScriptsHandle<SerializeFrom<typeof loader>> = {
    scripts(_params) {
        return [{
            src: "https://us.umami.is/script.js",
            defer: true,
            "website-id": "abcd",
            "data-domains": undefined
            }];
    },
};

However, this is rendered as:

<script src="https://us.umami.is/script.js" defer="" async=""></script>

Your Example Website or App

https://github.com/IgnisDa/ryot/blob/8204c2f5bc9de32e4181cf60a117db184b9eba88/apps/frontend/app/routes/_dashboard.tsx#L148

Steps to Reproduce the Bug or Issue

Expected behavior

I expect it to be rendered as:

<script src="https://us.umami.is/script.js" defer="" async="" data-website-id="abcd"></script>

Screenshots or Videos

No response

Platform

Additional context

No response

IgnisDa commented 6 months ago

I think this can be fixed by attaching rest props for this: https://github.com/sergiodxa/remix-utils/blob/e8a199133914a75fb4099e4b2356e73f65380e30/src/react/external-scripts.tsx#L229-L239

gar1t commented 4 months ago

Just a bump to this - not supporting data-xxx breaks this abstraction.

As a point of design feedback, I would not use rest/spread to support but an explicit property that simply passes through whatever is provided. E.g. extraAttrs or something like that. This is not a mainstream case, but it's a critical one.

wouterds commented 4 months ago

Would be nice, e.g. the Cloudflare beacon uses this custom data-cf-beacon attribute. You can have it automagically insert but then the console is full of hydration mismatch errors.

Edit: was just adding tracking and came across this, data attributes seem to be frequently used on third party scripts. Screenshot 2024-05-10 at 16 38 35

Rocinante89 commented 2 months ago

Agreed with @gar1t on this one. Came across this issue integrating Cloudflare beacon.

theosenoussaoui commented 1 month ago

Would love to the possibility of passing custom attributes as well !

theocerutti commented 1 month ago

same!