Add support for Shopify's Hydrogen/Oxygen
Description
Adds support for Shopify's Hydrogen and Oxygen project.
Changes:
Core
- replace deprecated usage of
require('url').parsewith a solution based onnew URL(...), verified that it behaves the same (for the most part) as the old function). Does have slightly different behavior for urls that don't have "slashes", for example: "http:example.com" will be parsed as "http://example.com", rather than being left as is. - fold the definition of
requestUrlto just usefetch. Remove custom fetch polyfill that usedhttpandhttps. - Type fixes for catch blocks where the value of
errorwas assumed to beany. Values are now coerced to anError. - Type error fix for
fetch({mode: ...}) - Fix issue with
fetchpolyfill. Changed to check iffetchis defined onglobalThisto better suppport all environments rather than using global, which is only available in NodeJS. - Changed to be a composite TS project.
React
- Fix issue with
fetchpolyfill. Changed to check iffetchis defined onglobalThisto better suppport all environments rather than using global, which is only available in NodeJS. - Type fixes for catch blocks where the value of
errorwas assumed to beany. Values are now coerced to anError. - Fix issue with
safeDynamicRequirewhich caused it to not function in the CloudFlare Worker environment. No longer assumes thatevalis available but rather checks ifrequireis a function, which should only be in NodeJS, and if that's the case it callseval('require'). I don't know if the eval trick is still required to work around webpack, or under what circumstances.
As an aside, in the future we should probably replace all usages of safeDynamicRequire with just just dynamic import(...), which should be supported everywhere now - it's at least supported in all LTS versions of NodeJS.
Glad to see you got this working! Now that I see it in actual code though, I am a little worried about the added complexity of having a rollup plugin versus changing the SDK code directly to work in all environments. We know that adding extra steps or complications acts as a roadblock for people to integrate, so ideally it would be nice if we could avoid requiring them to install a plugin to get builder to work in the oxygen environment. Plus the plugin would require us to maintain it as well as add additional docs for people to deal with.
On that note, forgive me if you already explained it to me (last week was a bit hectic sweat_smile ), but why do we need to use the rollup approach versus just modifying the functions directly to deal with the use of
eval? For example if we changedsafeDynamicRequireb690eab/packages/react/src/functions/safe-dynamic-require.ts to check for eval first (or also add a try catch if we need to), would that work and allow us to avoid having to maintain a separate plugin?export const safeDynamicRequire: typeof require = Builder.isServer && typeof eval === 'function' ? eval('require') : ((() => null) as any);or add in try catch something like
const getSafeRequire = () => { let safeDynamicRequire: typeof require; if (Builder.isServer && typeof eval === 'function') { try { safeDynamicRequire = eval('require') } catch (e) { } } return safeDynamicRequire || ((() => null) as any); }And then wherever we use this fancy require for
vm2, we could just returnnullwhenever the require is null, sincenullis a valid value for a binding.
~Unfortunately in code checks for environment specific quirks like typeof eval ... won't work on miniflare. When the miniflare app boots it checks for invaid or unsupported features during the parse phase, and fails to boot if they are present.~
~This is a minimal reproduction: https://github.com/Dkendal/miniflare-demo-nodejs/blob/main/index.js~
Looks like I was mistaken that cloudflare scans for code generation usages in the parse phase, it happens during runtime. Checking for the cf env is not great as you look for cache.default being defined but I can probably just change the sdk to have conditional branches for this like we do for react native
@mrkoreye this is ready for review again
description updated.