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

Packages used in middleware can't read env vars >=12.05 #33043

Closed
braden-clerk opened this issue Jan 5, 2022 · 5 comments · Fixed by #33141
Closed

Packages used in middleware can't read env vars >=12.05 #33043

braden-clerk opened this issue Jan 5, 2022 · 5 comments · Fixed by #33141
Labels
Middleware Related to Next.js Middleware.

Comments

@braden-clerk
Copy link

Run next info (available from version 12.0.8 and up)

No response

What version of Next.js are you using?

12.0.5

What version of Node.js are you using?

16.3.0

What browser are you using?

chrome

What operating system are you using?

macOS

How are you deploying your application?

yarn dev

Describe the Bug

Packages used in middleware return undefined for process.env.*

Expected Behavior

packages used in middleware should be able to read env vars

To Reproduce

https://github.com/braden-clerk/middleware-bug

This repo replicates the bug

@braden-clerk braden-clerk added the bug Issue was opened via the bug report template. label Jan 5, 2022
@balazsorban44 balazsorban44 added the Middleware Related to Next.js Middleware. label Jan 6, 2022
@balazsorban44 balazsorban44 removed the bug Issue was opened via the bug report template. label Jan 10, 2022
@kodiakhq kodiakhq bot closed this as completed in #33141 Jan 10, 2022
kodiakhq bot pushed a commit that referenced this issue Jan 10, 2022
After discussing with @sokra, seems that the proposed solution is split in two:

* We need to make sure that the `process` polyfill uses `global.process` if available. This is because middlewares are bundled using `browser` target and therefore `process.env.MY_ENV` gets shimmed into `require('process').env.MY_ENV`.

* Allow `process.env` to be statically analyzed for dependencies so they will be exported to the manifest.

Related issues:

* should fix #33043.
@ijjk
Copy link
Member

ijjk commented Jan 11, 2022

Hi, this has been updated in v12.0.8-canary.20 of Next.js, please update and give it a try!

@cj
Copy link

cj commented Jan 13, 2022

@ijjk I am using v12.0.8-canary.20 and still running into this issue

@cj
Copy link

cj commented Jan 13, 2022

I just tried v12.0.8 which was just released, the environment variables are still undefined inside of packages

@Schniz
Copy link
Contributor

Schniz commented Jan 13, 2022

Hey @cj can you please provide a repo with an example that fails? Seems like it is working for me: https://github.com/Schniz/next-test-3rd-party-env-vars 😃

@balazsorban44
Copy link
Member

This issue has been automatically locked due to no recent activity. If you are running into a similar issue, please create a new issue with the steps to reproduce. Thank you.

@vercel vercel locked as resolved and limited conversation to collaborators Feb 13, 2022
natew pushed a commit to natew/next.js that referenced this issue Feb 16, 2022
…l#33141)

After discussing with @sokra, seems that the proposed solution is split in two:

* We need to make sure that the `process` polyfill uses `global.process` if available. This is because middlewares are bundled using `browser` target and therefore `process.env.MY_ENV` gets shimmed into `require('process').env.MY_ENV`.

* Allow `process.env` to be statically analyzed for dependencies so they will be exported to the manifest.

Related issues:

* should fix vercel#33043.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Middleware Related to Next.js Middleware.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants