Minify JSON-LD markup result

[akshatmittal]

Description of Change(s):

JSON-LD Markup can be minified without any effect on parsing capabilities. This also solves the extra newlines issues present in most markups.

[riccardogiorato]

Do you have any benchmark or test to see the differences @akshatmittal! Love this fix, trying to make everything more small and fast!

[akshatmittal]

@riccardogiorato I don't have a benchmark, but I do have an example if you'd like. I use this plugin on one of my website and the current result looks something like this:

{
    "@context": "https://schema.org/",
    "@type": "Product",
    "image":["https://www.createmytoken.com/assets/social/primary.png"],
    "description": "Generate ERC20 Token on Ethereum and deploy it right from your browser in just 5 minutes!",
    
    
    
    
    
    
  "brand": {
      "@type": "Thing",
      "name": "Create My Token"
    },

    
    
  "aggregateRating": {
      "@type": "AggregateRating",
      
      "reviewCount": "230",
      
      "ratingValue": "4.58"
    },

    
    
    
    
    
    
    
    
    
    
    
    "name": "ERC20 Token Generator on Ethereum"
  }

Yep, as you can see it does have all of those whitespaces and what not in the markup itself. With this fix, the final result looks like this:

{"@context":"https://schema.org/","@type":"Product","image":["https://www.createmytoken.com/assets/social/primary.png"],"description":"Generate ERC20 Token on Ethereum and deploy it right from your browser in just 5 minutes!","brand":{"@type":"Thing","name":"Create My Token"},"aggregateRating":{"@type":"AggregateRating","reviewCount":"230","ratingValue":"4.58"},"name":"ERC20 Token Generator on Ethereum"}

And all of this with no impact on how search engines and bots would parse it!

[garmeeh]

Hey @akshatmittal thanks for the PR.

We did have a feature like this live and had to revert due to it causing issues.

JSON.stringify will not work as expected unfortunately. Your previous .map solution was an interesting approach.

I do wonder what is the performance cost of the browser parsing extra newlines versus having to run JavaScript such as .map or JSON.stringify 🤔

You can see the original PR here: #487
And revert here with issues linked: #540

I am open to solving this issue, just need to make sure the fix is valid and not counter productive. 😅

[akshatmittal]

Hey @garmeeh, thanks for that! I didn't know that existed but was reverted. I definitely think this is valuable and both of the bugs linked are caused by unhandled escapes. I will probably look deeper into this in a little bit (yes, I really want the minified JsonLd).

And yea, that .map is a neat little trick I often use to parse out files and stuff. Somehow that was my first thought before using the JSON parser. I think the JSON parser is for sure slower than the .map approach, but would love to be proven wrong on that one, although I do think both of them are sufficiently fast enough for the purpose.

[akshatmittal]

@garmeeh So as it turns out, neither of the linked bugs are actually minify bugs to begin with. No text in the entire codebase is ever escaped, and directly creates a string instead of using JSON objects. Whenever quotes are passed to any component, the returning string is not valid JSON. This would automatically be solved by using JSON objects instead of string builders. Minification will also work fine with JSON.stringify. Funnily that would also save you from things like ${sku ? `"sku": "${sku}",` : ''} since undefined is never rendered back into JSON and also minify the code (nice).

At the moment, my recommendation would be to not merge this change. The .map approach will strip out all newlines, which is obviously a problem on it's own, and the current approach validates the JSON which is invalid in a bunch of cases.

I do think it would be quite worth it to rewrite the entire library to use Objects instead of string builders.

Edit: Going back, it looks like the unescaped strings are causing issues for a lot of other people as well.

[garmeeh]

Thanks for the insight @akshatmittal. Yeah when I first built next-seo it was under the assumption it would only be passed valid data. But in hindsight this might this may have been a bad choice 😅

In your mind what would it look like using JSON objects instead? Maybe you could give a small example of what you are thinking?

I can try evaluate the effort in re-writing and also if it can be done incrementally. There are few other things that I could tackle during the re-write also.

[akshatmittal]

In your mind what would it look like using JSON objects instead? Maybe you could give a small example of what you are thinking?

Here's an example @garmeeh, the current socialProfile.tsx can be re-written as:

const jslonld = {
    "@context": "https://schema.org",
    "@type": type,
    "name": name,
    "url": url,
    "sameAs": sameAs,
};

Then passed on as JSON.stringify(jslonld), or instead (as I'd prefer) pass it as an object itself, and finally stringify it in markup.tsx instead.

Any variable not passed to the React component will be undefined and hence automatically stripped out from the JSON. You can also always filter out null keys if you absolutely need to. This will automatically escape any input as well.

The effort is definitely significant since it basically replaces ALL the files, but can be done incrementally. If you do plan on tackling other issues, I'd also highly recommend strongly typing the components.

[fabien]

Definitely interested in this feature! Using JSON objects would also allow for more extensibility. Currently, the schemas can be a bit limiting, and in case some additional properties are needed, you're out of luck.

[garmeeh]

Thanks @akshatmittal. I really like this approach and it will simplify things. I will carry out a test on this approach next week and see what will be involved in re-writing.

[garmeeh]

Ok so I have tested this out and it works pretty well. I am currently planning out the re-write as there are other changes I would like to make and upgrades to the project codebase I want to make at the same time.

I am pretty busy at work at the moment and have some holidays coming up but will try my best to get this out ASAP.

[gregg-cbs]

@garmeeh

If you need a hand, someone to help with the change over I can get on a call with you one day and help you tackle some files?
You let me know.

[garmeeh]

Thanks everyone for baring with me on this one. 😅 next-seo has been fully re-written and I need to fully verify but quickly checking the ProductJsonLd and the output is now the following:

{"@context":"https://schema.org","@type":"Product","description":"Sleeker than ACME's Classic Anvil, the Executive Anvil is perfect for the business traveler looking for something to drop from a height.","color":"blue","material":"steel","slogan":"For the business traveller looking for something to drop from a height.","disambiguatingDescription":"Executive Anvil, perfect for the business traveller.","releaseDate":"2014-02-05T08:00:00+08:00","productionDate":"2015-02-05T08:00:00+08:00","purchaseDate":"2015-02-06T08:00:00+08:00","award":"Best Executive Anvil Award.","mpn":"925872","image":["https://example.com/photos/1x1/photo.jpg","https://example.com/photos/4x3/photo.jpg","https://example.com/photos/16x9/photo.jpg"],"brand":{"@type":"Brand","name":"ACME"},"review":[{"datePublished":"2017-01-06T03:37:40Z","reviewBody":"This is my favorite product yet! Thanks Nate for the example products and reviews.","name":"So awesome!!!","@type":"Review","author":{"@type":"Person","name":"Jim"},"reviewRating":{"bestRating":"5","ratingValue":"5","worstRating":"1","@type":"Rating"},"publisher":{"@type":"Organization","name":"TwoVit"}}],"aggregateRating":{"@type":"AggregateRating","reviewCount":"89","ratingValue":"4.4"},"manufacturer":{"@type":"Organization","name":"Gary Meehan","logo":{"@type":"ImageObject","url":"https://www.example.com/photos/logo.jpg"}},"offers":[{"price":"119.99","priceCurrency":"USD","priceValidUntil":"2020-11-05","itemCondition":"https://schema.org/UsedCondition","availability":"https://schema.org/InStock","url":"https://www.example.com/executive-anvil","@type":"Offer","seller":{"@type":"Organization","name":"Executive Objects"}},{"price":"139.99","priceCurrency":"CAD","priceValidUntil":"2020-09-05","itemCondition":"https://schema.org/UsedCondition","availability":"https://schema.org/InStock","url":"https://www.example.ca/executive-anvil","@type":"Offer","seller":{"@type":"Organization","name":"Executive Objects"}}],"name":"Executive Anvil"}