stereobooster / react-snap

👻 Zero-configuration framework-agnostic static prerendering for SPAs
MIT License
5.04k stars 391 forks source link

CDN support #92

Open stereobooster opened 6 years ago

stereobooster commented 6 years ago

Related: https://github.com/chrisvfritz/prerender-spa-plugin/issues/114

config can look like:

"reactSnap": {
  "cdn": {
    "https://mysite.mycdn.com/assets": "build/assets"
  }
}

And code can be like

await page.setRequestInterception(true)
page.on('request', request => {
  if (request.url.startsWith(cdnUrl)) {
    // serve file from local file system instead of network
  }
})

Right now I’m wary of increasing surface area for bugs. I don’t want to make this as complicated as WDS client. I still plan to make changes to it, so I intend to keep it scoped to features CRA supports, and serving JS from a different host or port is not supported right now.

CRA on CDN_URL

kirkchris commented 6 years ago

Would love to have this. Right now we're building the site and uploading new assets to CDN and then just loading them remotely during our snapshotting. However this introduces some other issues because I'd like to use skipThirdPartyRequests and am unable to do so because our CDN url is a third party. Not skipping third party requests right now is causing some weirdness with stripe JS.

Another potential enhancement that is somewhat different, but slightly related, would be to enable skipThirdPartyRequests to support a new option of skipThirdPartyRequestsExcept as an array (or flipped baseName as an array) that way we can conditionally allow some third party requests to be loaded as needed. Happy to create a separate issue tagged under enhancement for this.

If you have any ideas on how to do this with the current version, would love to hear.

Great work on the package!

stereobooster commented 6 years ago

skipThirdPartyRequestsExcept doesn't sound good to me

kirkchris commented 6 years ago

I'm very open on naming if that's what you meant...

stereobooster commented 6 years ago

I mean it does not feel right. If something like this starts to appear it means that abstraction is wrong - either tool applied to wrong task or tool is wrong. If the original idea will fix your issue we should do that

kirkchris commented 6 years ago

Yeah, I understand what you mean. It gets us close, however we also have a section of our site on the public facing area (that we are snapshotting) that loads content in from the Contentful CMS. So there is a case in which I want to allow some third party requests but not others. Potentially for my use case, I need to just allow third party requests and strip out the stripe.js loader during snapshotting, then add it back in to HTML afterwards.

stereobooster commented 6 years ago

Thinking out loud.

There is also proxy from create-react-app (#101), which translates path to url and works on the server.

Proposed cdn config translates url to path and works in the browser.

It can also translate from url to url - in our case, it will translate from given url to exactly the same url and can be expressed as:

"reactSnap": {
  "cdn": {
    "https://mysite.mycdn.com/assets": "build/assets", // url -> paht
    "https://api.contentfull.com": true // url -> url
  }

But the name cdn doesn't fit in this case. Also option cdn will always take precedence over skipThirdPartyRequests and will work independently of skipThirdPartyRequests config.

kirkchris commented 6 years ago

That could definitely work. Agree that cdn should work independently of skipThirdPartyRequests as it's separate and actually by nature, bypassing something even being a third party request.

I think the name proxy could even be a better name than cdn in config, as that's really what this does and also opens up for more flexibility, as you showed above. Plus allows for support if they had multiple CDN urls for different types of content or something.

kirkchris commented 6 years ago

Created a PR (https://github.com/stereobooster/react-snap/pull/136) with the changes discussed. Happy to continue the discussion and make changes.

stereobooster commented 6 years ago

It can happen that I over complicated the task. Maybe publicUrl would be enough, which would correspond to Create React App %PUBLIC_URL%. What is the case for multiple urls?

Related:

baptooo commented 5 years ago

Hello @stereobooster thank you for the library it really helps !

For this subject I just want to share with you a (bit dirty...) hack I found to be able to prevent some scripts from loading while ReactSnap is crawling

<boby>

  <div id="third-party-container"></div>
  <script>
    if (navigator.userAgent === 'ReactSnap')  {
      document.write('<textarea id="third-party">');
    }
  </script>
  <script src="https://example.com/third-party-script.js"></script>
  </textarea>

  <script>
    window.snapSaveState = () => {
      document.getElementById('third-party-container').innerHTML =
        document.getElementById('third-party').value;
    }
  </script>
</body>

To explain a bit:

The main idea is to condition the scripts with a <textarea /> element that will be enabled only when ReactSnap is crawling.

So for the following cases:

Local dev server The <textarea /> won't be written so the scripts will load

Crawling with react-snap The <textarea /> will be written until the page is fully crawled and then the snapSaveState callback will move the scripts to third-party-container and remove the <textarea /> to prevent from polluting the HTML

Snapshot built The scripts will be inside third-party-container.

Caveat

The main issue is that your app built without react-snap will have a closing </textarea> tag that will invalidate your HTML...