Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Environment variables and bindings on env #1

Closed
GregBrimble opened this issue Oct 20, 2022 · 28 comments · Fixed by #58
Closed

Environment variables and bindings on env #1

GregBrimble opened this issue Oct 20, 2022 · 28 comments · Fixed by #58

Comments

@GregBrimble
Copy link
Member

Next.js has it's own API design for getServerSideProps() and the Edge API Routes and Middleware take only the NextRequest object as its single parameter. This means that passing Workers' env object which contains environment variables and bindings isn't immediately obvious.

One idea is to 'pollute' the global space and access things by request. This has the advantage of being similar to how it works today, but might be too far into the uncanny valley.

For example:

import type { NextRequest } from 'next/server'

export const config = {
  runtime: 'experimental-edge'
}

export default async function(req: NextRequest) {
  const value = process.env[req].MY_ENV_VAR
}

Another might be to use the @cloudflare/next-on-pages package as a runtime library which is different enough to be intentional, but still fundamentally works the same way:

import type { NextRequest } from 'next/server'
import { env } from '@cloudflare/next-on-pages'

export const config = {
  runtime: 'experimental-edge'
}

export default async function(req: NextRequest) {
  const value = env(req, 'MY_ENV_VAR')
}

What do y'all think? Any preferences? Any other ideas?

@GregBrimble GregBrimble pinned this issue Oct 20, 2022
@bluebeel
Copy link

bluebeel commented Oct 24, 2022

Why not extend NextRequest?
Next ignores the properties added to the object. So we can add what we want to the Request object. Then we just have to provide a type CfNextRequest = NextRequest & { env: Record<string, unknown> } via the @cloudflare/next-on-pages lib.

export default {
  async fetch(request, env, context) {
    const { pathname } = new URL(request.url);
    const routes = routesMatcher({ request }, __CONFIG__.routes);

   // link env to request object
   request.env = env;

    for (const route of routes) {
      if ("middlewarePath" in route && route.middlewarePath in __MIDDLEWARE__) {
        return await __MIDDLEWARE__[route.middlewarePath].entrypoint.default(
          request,
          context
        );
      }
    }

    for (const { matchers, entrypoint } of Object.values(__FUNCTIONS__)) {
      let found = false;
      for (const matcher of matchers) {
        if (matcher.regexp) {
          if (pathname.match(new RegExp(matcher?.regexp))) {
            found = true;
            break;
          }
        }
      }

      if (found) {
        return entrypoint.default(request, context);
      }
    }

    return env.ASSETS.fetch(request);
  },
} as ExportedHandler<{ ASSETS: Fetcher }>;

and we have

import { CfNextRequest } from '@cloudflare/next-on-pages'

export const config = {
  runtime: 'experimental-edge'
}

export default async function(req: CfNextRequest) {
  const value = req.env.MY_ENV_VAR
}

@mrkstwrt
Copy link

mrkstwrt commented Nov 4, 2022

Any plans to add env to this package soon or are pull request along the lines of @bluebeel's suggestion welcome? This is a pretty critical feature in my opinion.

@kamranayub
Copy link

For the life of me I cannot figure out how to get geo info (req.cf in the worker API). I wonder if this is related?

@derekyau
Copy link

What is the current way in which you access ENV variables in a NextJS experimental-edge app on Cloudflare? I can't seem to figure this out...

@Hades32
Copy link

Hades32 commented Nov 20, 2022

@mrkstwrt I see you made a fork with that change. Did it work for you? It doesn't seem to do anything for me...

@Hades32
Copy link

Hades32 commented Nov 20, 2022

@GregBrimble this is super frustrating. Just wasted a few hours trying to get this to work as it makes Functions basically useless to me. Is there any workaround at the moment?

@luca-rath
Copy link

Hi @Hades32,
I just stumbled across the same issue but I've found a workaround.
If you adjust your next.config.js like in the following example, you can use process.env.API_KEY and process.env.OTHER_SECRET in your edge api routes.

const { EnvironmentPlugin } = require('webpack');

/** @type {import('next').NextConfig} */
const nextConfig = {
    // ...
    webpack(config) {
        config.plugins.push(new EnvironmentPlugin(['API_KEY', 'OTHER_SECRET']));

        return config;
    },
};

@mrkstwrt
Copy link

Hi @Hades32,
I just stumbled across the same issue but I've found a workaround.
If you adjust your next.config.js like in the following example, you can use process.env.API_KEY and process.env.OTHER_SECRET in your edge api routes.

const { EnvironmentPlugin } = require('webpack');

/** @type {import('next').NextConfig} */
const nextConfig = {
    // ...
    webpack(config) {
        config.plugins.push(new EnvironmentPlugin(['API_KEY', 'OTHER_SECRET']));

        return config;
    },
};

Isn't that the same as using the env config and will expose all the provided secrets in the frontend bundle too?

@luca-rath
Copy link

@mrkstwrt Thanks for the hint! To be honest, I didn't think of that 😅 But I just checked, as long as you don't put process.env.API_KEY in any of your client components, it won't be included in the client bundle. So I'll continue to use it, but I totally agree that there should be a better solution for this.

@Hades32
Copy link

Hades32 commented Nov 25, 2022

@luca-rath thanks, but does that really work for binding, too? That's the real issue I'm facing

@hyunjunian
Copy link

I am very much looking forward to this.

@muslax
Copy link

muslax commented Nov 25, 2022

Wow, have been waiting for this since Next 13 release. Thanks @luca-rath and you all. 🚀

@muslax
Copy link

muslax commented Nov 25, 2022

For me still doesn't work for binding... 😢

@jokull
Copy link

jokull commented Dec 2, 2022

Here's the changes needed. Hopefully this person makes a PR, this is very valuable. main...YuheiNakasaka:next-on-pages:feature/add-env-to-functions

@mountainash
Copy link

Here's the changes needed. Hopefully this person makes a PR, this is very valuable. main...YuheiNakasaka:next-on-pages:feature/add-env-to-functions

Tried to run this in the build process (using the Github Action cloudflare/pages-action@1) with "@cloudflare/next-on-pages": "github:YuheiNakasaka/next-on-pages#feature/add-env-to-functions" in the devDependencies of the package.json.

Exit code: 128
info Visit https://yarnpkg.com/en/docs/cli/install for documentation about this command.
Command: git
Arguments: ls-remote --tags --heads ssh://git@github.com/YuheiNakasaka/next-on-pages.git
Directory: /home/runner/work/{PROJECT}/{REPO}
Output:
git@github.com: Permission denied (publickey).
fatal: Could not read from remote repository.

Please make sure you have the correct access rights
and the repository exists.
Error: Process completed with exit code 128.

😞

@jokull
Copy link

jokull commented Dec 6, 2022

Yeah apparently this doesn't work -spoke with the author on Twitter. Excited about next-on-pages supporting this. Until then I'm hosting trpc server on Cloudflare and next on vercel. Monorepo.

@scrummitch
Copy link

Super frustrating that this isn't included. I would expect all my defined environment variables to be available in process.env

@kingrupam
Copy link

@GregBrimble please tell me key value is implemented or not and geo location or req.body.cf implemented or not

@GregBrimble
Copy link
Member Author

GregBrimble commented Jan 25, 2023

Hey folks! 👋

We've just merged a PR (#58) which will add the ability to access environment variables and bindings from within your SSR pages/API handlers. This will be available with the next release (@cloudflare/next-on-pages@0.3.0). We expect this to be released by the end of the week. You can be notified by subscribing to this PR: #60.

Apologies for it taking so long! We know it was a point of frustration for a lot of people. It doesn't quite make up for that, but here's part of the reason why:

1. Putting the Cloudflare Workers environment on process.env

Where normally within a Worker, you'd access your environment like so:

export default {
  async fetch(request, env, ctx) {
    // `env` is an object with your bindings

    // To access simple environment variables/secrets
    console.log(env.MY_ENV_VAR)

    // To access the value of `my-key` in the `MY_KV_NAMESPACE_BINDING` KV namespace
    console.log(await env.MY_KV_NAMESPACE_BINDING.get('my-key'))

    // etc.
  }
}

Next.js does not have a built-in way to pass this env object to the various module functions (unlike other frameworks such as Remix and Qwik). Instead, since Next.js started as a project for Node.js, it relies on Node.js' process.env global.

export async function getServerSideProps() {

  // `process.env` is an `Record<string, string>` object with the system environment variables loaded on it.
  console.log(process.env.MY_ENV_VAR)

  // etc.
}

Naturally, this process.env object doesn't usually have special bindings to things like KV namespaces on it. However, that's what we've done here, to make the bridge between Next.js and the Cloudflare Workers/Pages platform as familiar as possible to both sides.

export async function getServerSideProps() {
  // `process.env` is now just that `env` you'd typically get in the `fetch(request, env, ctx)` handler of a Worker.
  // So, as above...

  // To access simple environment variables/secrets
  console.log(process.env.MY_ENV_VAR)

  // To access the value of `my-key` in the `MY_KV_NAMESPACE_BINDING` KV namespace
  console.log(await process.env.MY_KV_NAMESPACE_BINDING.get('my-key'))

  // etc.
}

We were hesitant to add this on day 1 because we re-use isolates as a performance optimization wherever possible. This means that as we process one request with set of environment bindings, another request also start to be processed, potentially with a different set of environment bindings. In the typical Cloudflare Worker written in ESM format, this isn't an issue since the request and env come in together. However, if you were to access the environment from a global scope like Next.js does, there's a risk that the new set of environment bindings will override the previous set as that previous request is still processing.

To be clear, we do not share isolates across accounts (or even, across different Workers), so there's never as risk of your environment leaking across accounts or projects. Instead, if you updated your environment variables of your project as it served live traffic, we were concerned that potentially some requests would be 'polluted' with the newer set of environment variables as they were still being processed.

It turns out, that after some investigation, the way that Pages does deployments means that this won't happen anyway. It took us some time to confirm this, but we're now confident that there will be no accidental 'leakage' of environment bindings across deployments.

However, if you choose to set things on the process.env object from within the code of your Next.js project, you may see this 'leak' into other requests. This is true for any values you set in the global scope of any Cloudflare Worker. The isolate will eventually be evicted and those values will eventually be lost, but they may persist while the isolate remains live. For example:

export async function getServerSideProps() {
  // The very first time the isolate processes traffic, this would be empty (assuming I don't have any environment variables configured on my project)
  console.log(process.env.MY_TEMPORARY_VARIABLE)

  // If I then set some value on it though...
  process.env.MY_TEMPORARY_VARIABLE = 'test'

  // This will be available to the rest of my application as I process this request
  console.log(process.env.MY_TEMPORARY_VARIABLE) // 'test'

  // But also, if this isolate is re-used for a second request, the first `console.log` _may_ now print 'test'.
  // This is possible if the request hits the same Cloudflare location, but is in no way guaranteed.
  // The same is true if I set a value like so:

  globalThis.OTHER_TEMPORARY_VARIABLE = 'test'
}

You can test this behavior for yourself in the Cloudflare Workers playground. Generally, abusing the global scope like this is an anti-pattern. It will cause bugs in your apps unless you really understand what this means and so, generally, you should try and stay away from putting temporary values in the global scope whenever possible.

2. Bundling

Today, this @cloudflare/next-on-pages adaptor produces an "Advanced Mode" _worker.js. This Worker doesn't currently support bundling or any other sort of processing by Wrangler, which is needed for D1 support while it's in open alpha. D1 works today by shimming in some JavaScript to make the binding API work seamlessly over what is otherwise a .fetch() contract. If you're interested in the nitty gritty, you can see the facade in the Wrangler 2 codebase. We're not quite there yet with this, but you can expect D1 bindings to work soon. We need to make some changes in Wrangler, release those (which will make it work in wrangler pages dev and for any Direct Upload projects using wrangler pages publish) and then update our internal version of Wrangler which we use in the CI builders for any git-connected projects (GitHub/GitLab). I'll update #61 as we proceed with this work.

3. AsyncLocalStorage

Node.js support is coming to Cloudflare Workers. We are actively building out some of the Node.js APIs, and some of them are available already if you're using workerd directly. You can enable them with --experimental and the nodejs_compat compatibility flag. It's not yet available in production, but once we figure out how best to offer this opt-in, it will be.

Specifically, the AsyncLocalStorage API from node:async_hooks is interesting for this @cloudflare/next-on-pages project, and it's one of the first that has been completed by this compatibility effort. Next.js have already started exploring use of the AsyncLocalStorage API directly in their framework, and it's possible that we may be able to do this process.env implementation "properly" without leakage with these new APIs once they're available.

4. Pages Functions GA and other work

The Pages team was very busy at the end of last year getting Pages Functions ready for general availability. We'd love to give 100% to every project all the time, but hopefully it is understandable that sometimes we have to prioritize some projects while others take a bit of a backseat. However, we appreciate that @cloudflare/next-on-pages was a particular frustration and we'll endeavor to be more regular in our updates here going forward. Please remember that ultimately, we love building useful products and features for the community — that's what we spend practically every day thinking about! Please continue to create issues in relevant GitHub repos, vote on them, and to post in our Discord. We do see your feedback and we absolutely do consider it when planning our roadmap.

I hope this is all makes sense.


So, to summarize here:

  • @cloudflare/next-on-pages@0.3.0 will be released soon — follow along here: Version Packages #60.
  • process.env will contain the environment object you're used to seeing in Cloudflare Workers, populated with the bindings of your project.
  • D1 bindings won't work immediately, but you can expect them to follow soon.
  • Node.js compatibility, including async_hooks's AsyncLocalStorage can tested in workerd today, and it will be coming to Cloudflare Workers.

@Cherry
Copy link

Cherry commented Jan 25, 2023

This is fantastic to hear, thanks @GregBrimble. 🎉

One of the other largest blockers for people in my experience is the output size, as documented at #47. It's regularly reported in the Discord too, and also contributes to issues like #54. Seeing some movement on these in the near future would be awesome.

@muslax
Copy link

muslax commented Jan 29, 2023

Thanks @GregBrimble.

@mountainash
Copy link

Just saw that v0.3.0 was released. I've just re-built and deployed my project that was using process.env to get some key/secrets and confirm that it works. Bug #1 is fixed 🎉. Thanks @GregBrimble !

@mikecentric
Copy link

I have tested and can confirm env vars are working as expected.

@GregBrimble GregBrimble unpinned this issue Feb 21, 2023
@mikecentric
Copy link

Hey guys seems bindings work fine after you deploy.

Anyone know how to get them to work locally?

@mountainash
Copy link

Anyone know how to get them to work locally?

They need to be in your environment/session firstly, then you can call wrangler pages dev with the "binding" parameter like --binding VAR_1=$VAR_1 VAR_2=$VAR_2 etc.

@willin
Copy link

willin commented May 18, 2023

▲  Error occurred prerendering page "/api/hello". Read more: https://nextjs.org/docs/messages/prerender-error
▲  TypeError: Cannot read properties of undefined (reading 'get')
▲  at GET (/Users/v0/Projects/willin/free-domain/.next/server/app/api/hello/route.js:71:32)
import { KVNamespace } from '@cloudflare/workers-types';
import { NextResponse } from 'next/server';

export async function GET(request: Request) {
  const { DOMAINS } = process.env as any as { DOMAINS: KVNamespace };
  const data = await DOMAINS.get('test'); // throw error when build
  return NextResponse.json({ data });
}

@willin
Copy link

willin commented May 18, 2023

import { KVNamespace } from '@cloudflare/workers-types';
import { NextResponse } from 'next/server';

export async function GET(request: Request) {
  const { DOMAINS } = process.env as any as { DOMAINS: KVNamespace };
  await DOMAINS?.put('test', 'value');

  const data = await DOMAINS?.get('test');
  return NextResponse.json({ data });
}

codes above can be built. but got: {}

run cmd: npx wrangler@3 pages dev .vercel/output/static --kv=DOMAINS --compatibility-flag=nodejs_compat

@dario-piotrowicz
Copy link
Member

@willin I think that process.env for KVs works just fine 🤔

I just created this app to test it out: kv-app-dir-13.4.4

I run it locally with the pages:dev script
and everything seems to be working as expected:
Screenshot 2023-05-26 at 13 59 07

So I am not sure what issues you're encountering, could you provide more details? if you could provide a minimal reproduction please do so as that would really help 🙂
(also if you want feel free to open a new issue as conversations in closed issues are easy to get lost/forgotten)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.