s-yadav / react-meta-tags

Handle document meta/head tags in isomorphic react with ease.
MIT License
226 stars 47 forks source link

fix(meta_tags): add special case for dealing with json-ld scripts #35

Open bertyhell opened 4 years ago

bertyhell commented 4 years ago

closes: https://github.com/s-yadav/react-meta-tags/issues/34

Removes all instances of <script type="application/ld+json"> before adding a new one

before this PR: image

after this PR: image

bertyhell commented 4 years ago

@s-yadav any chance to merge this?

oles-bolotniuk commented 3 years ago

@s-yadav ping

s-yadav commented 3 years ago

@oles-bolotniuk @bertyhell Can't there be multiple json-ld scripts in a page? I guess this should be handled with script tag check and id attribute.

If there can be only one json-ld script in a page, then the changes looks good.

bertyhell commented 3 years ago

It looks like multiple json-ld scripts are allowed according to the spec: https://w3c.github.io/json-ld-syntax/#embedding-json-ld-in-html-documents

alexander-morgunov commented 3 years ago

@bertyhell the problem is that in my case, I need to update tag while navigate to another page. In current solution - I'll get new tag on each route change, without removing old ones.

s-yadav commented 3 years ago

@bertyhell I guess then we can handle it based on id. The library already checks for id to identify uniqueness. But the script is not in the inclusion list. We can include script tag as well for this.

On other note, only non-js scripts make sense to be kept inside meta tags. For normal JavaScript scripts once the methods/variables are added to global scope it can't be removed so it doesn't make to handle update for those.

bertyhell commented 3 years ago

for anyone blocked by this issue, for now you can just include your own component with your own rules for deleting other tags:

import { FunctionComponent } from 'react';

export interface JsonLdProps {
    url: string;
    title?: string;
    description?: string | null;
}

const JsonLd: FunctionComponent<JsonLdProps> = ({
    url,
    title,
    description,
}) => {
    document
        .querySelectorAll('script[type="application/ld+json"]')
        .forEach((script) => script.remove());

    const info = {
        '@context': 'https://schema.org',
        '@type': 'Article',
        mainEntityOfPage: {
            '@type': 'WebPage',
            '@id': url,
        },
        headline: title || '',
        description: description || '',
    };

    const scriptElem = document.createElement('script');
    scriptElem.setAttribute('type', 'application/ld+json');
    scriptElem.innerHTML = JSON.stringify(info, null, 2);
    document.head.append(scriptElem);
    return null;
};

export default JsonLd;

The relevant line for deleting other scripts is:

    document
        .querySelectorAll('script[type="application/ld+json"]')
        .forEach((script) => script.remove());

you could for instance only delete tags with a specific id:

    document
        .querySelectorAll('script[type="application/ld+json"][id="mySpecificId"]')
        .forEach((script) => script.remove());