Use browser-friendly `process.env.*`

[davidmurdoch]

The Node.js testing code left in the distributed SDK can cause problems with bundlers that don't need to mock Node's process emulation or process.env.* replacement (MetaMask is using one of these "lazy" bundlers).

gridplus-sdk/packages/sdk/src/util.ts

Lines 377 to 378 in 6ca05ad

	const apiKey = process.env.ETHERSCAN_KEY;
	const apiKeyParam = apiKey ? `&apiKey=${process.env.ETHERSCAN_KEY}` : '';

A potential fix:

function buildUrlForSupportedChainAndAddress({ supportedChain, address }) {
  const baseUrl = supportedChain.baseUrl;
  const apiRoute = supportedChain.apiRoute;
  const urlWithRoute = `${baseUrl}/${apiRoute}&address=${address}`;

  const apiKey = globalThis.process?.env?.ETHERSCAN_KEY;

  const apiKeyParam = apiKey ? `&apiKey=${apiKey}` : '';
  return urlWithRoute + apiKeyParam;
}

Since this looks to be an undocumented env var, I don't think this would break any contracts with code that depends on this behavior. However, there is a change this could break some "dumb" bundlers that use naive string/regex replacement, instead of AST-based replacemers.

If you need to support verbatim process.env.* something like this will likely work:

  const apiKey =
    (typeof globalThis !== "undefined" &&
      globalThis.process &&
      globalThis.process.env &&
      globalThis.process.env.ETHERSCAN_KEY) || 
    "";

  const apiKeyParam = apiKey ? `&apiKey=${apiKey}` : "";

There are probably other instances of env var injection left in the code that could use a similar "fix".

Of course, the most ideal situation is for code meant for testing would just be compiled out before publishing :-)

Let me know if you want either of these fixes, or something else, and I can open a PR for it.

[yilmazbahadir]

Thanks @davidmurdoch for taking the time to report this. I’ll dig into it next sprint and check for any other instances too.