remove jsdom for prod pipeline

[thescientist13]

Related Issue

resolves #138

Summary of Changes

1. Removed "duplicate" serialization by removing JSDOM from the build pipeline.

Takes a nice chunk out of our dependency footprint

$ yarn why jsdom
yarn why v1.12.3
[1/4] 🤔  Why do we have the module "jsdom"...?
[2/4] 🚚  Initialising dependency graph...
[3/4] 🔍  Finding dependency...
[4/4] 🚡  Calculating file sizes...
=> Found "[email protected]"
info Has been hoisted to "jsdom"
info This module exists because it's specified in "devDependencies".
info Disk size without dependencies: "3.12MB"
info Disk size with unique dependencies: "7.31MB"
info Disk size with transitive dependencies: "19.57MB"
info Number of shared dependencies: 51
✨  Done in 0.91s.

[thescientist13]

As far as I can tell, the output is the same.

[hutchgrant]

There was a specific reason for using jsdom, something did not serialize properly. Extra caution with this.

[thescientist13]

There was a specific reason for using jsdom, something did not serialize properly.

What was it?

[hutchgrant]

const dom = new JSDOM(``, {
  url: "https://example.org/",
  referrer: "https://example.com/",
  contentType: "text/html",
  includeNodeLocations: true,
  storageQuota: 10000000
});

url sets the value returned by window.location, document.URL, and document.documentURI, and affects things like resolution of relative URLs within the document and the same-origin restrictions and referrer used while fetching subresources. It defaults to "about:blank".

Removing this line could potential cause issues related to path/url of resources. I do not know where specifically this was an issue previously.

[hutchgrant]

Although from my cursory tests I haven't seen any issues.

[hutchgrant]

As per the issue cited in the comment: jsdom/jsdom#2005

<style type="text/css">
@import url("/css/blok_add.css");
</style>

Cannot be parsed without the correct url. Although I do not see that error serializing in puppeteer, maybe because we don't have any examples of relative css imports.

With jsdom:

The problem can be solved by specifying the parameter url.

new JSDOM(html, {url: 'http://example.com'})

[hutchgrant]

https://github.com/hutchgrant/ssr-evergreen-demo/blob/b18f31038dd24fb1e003ffdb00a18a20429e1d6c/renderer.js#L31

From my previous SSR-demo which formed the basis of this code base, I previously had puppeteer serialize, then used JSDOM to manipulate dom, then serialized again.

Perhaps thats why we kept it? In case we needed to change dom post-serialization via puppeteer?

The resulting page can now be modified easily with anything you need to be done on the server via JSDOM, serialized, and sent back to the client.

[thescientist13]

Gotta love unit tests 😌

As part of the PR that added that comment re: JSDOM, we also added unit tests specifically for @import. This was for issue #134

All unit tests are still passing and the website preview is working so I think all the right checks and balances are in place, IMO.

[thescientist13]

In general, removing an extra translation layer seems like the right move architecturally too. If we're already pre-rendering / serializing with a "real" browser, serializing it through a fake one on top of that seems unnecessary.

[thescientist13]

That said, if there are specific test cases we need to validate, please suggest them and I can look to see if they are being covered.

[thescientist13]

going to repoint this to release branch to avoid issues with packaging changes due to monorepo