-
Notifications
You must be signed in to change notification settings - Fork 46
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(meta_tags): add special case for dealing with json-ld scripts #35
base: master
Are you sure you want to change the base?
fix(meta_tags): add special case for dealing with json-ld scripts #35
Conversation
@s-yadav any chance to merge this? |
@s-yadav ping |
@oles-bolotniuk @bertyhell Can't there be multiple json-ld scripts in a page? If there can be only one json-ld script in a page, then the changes looks good. |
It looks like multiple json-ld scripts are allowed according to the spec: |
@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. |
@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. |
for anyone blocked by this issue, for now you can just include your own component with your own rules for deleting other tags:
The relevant line for deleting other scripts is:
you could for instance only delete tags with a specific id:
|
closes: #34
Removes all instances of
<script type="application/ld+json">
before adding a new onebefore this PR:

after this PR:
