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

Production serverless build importing Firestore fails while including 'google/protobuf/api.proto' #11860

Closed
WestonThayer opened this issue Apr 13, 2020 · 16 comments · Fixed by #11861

Comments

@WestonThayer
Copy link
Contributor

Bug report

Describe the bug

When using import "firebase/firestore" on a page with target: "serverless" in next.config.js, next build fails with the following error:

> Build error occurred
Error: ENOENT: no such file or directory, open 'google/protobuf/api.proto'

To Reproduce

  1. Clone https://github.com/WestonThayer/bug-nextjs-firestore-grpc
  2. npm install
  3. npm run build

Expected behavior

The production build should succeed. It used to work in next@8.1.0.

System information

  • OS: macOS 10.14.6 (Mojave)
  • Version of Next.js: 9.3.4
  • Version of Node.js: v12.15.0
  • Version of Firebase: 7.14.0

Additional context

  • The build will succeed if you remove target: "serverless" from next.config.js
  • Strangely, next build succeeds in the Zeit NOW environment. I see NOW is also using Node v12.15.0, so scratching my head as to what's different

Seems like this is something that Next tried to get working in the past:

@WestonThayer
Copy link
Contributor Author

My understanding of this issue is that target: "serverless" tells Next to use Webpack to bundle each page & dependencies into a single serverless function file. The .proto files are not getting bundled by Webpack (probably because there's not a loader for them).

It looks like this has caused a variety of issues.

What I don't understand is why it works in next@8.1.0...

@WestonThayer
Copy link
Contributor Author

I can confirm that tweaking grpc by no longer requiring the .proto files allows the build to succeed (example). It appears to work fine, but I'm not sure what the .proto files are used for.

@Timer
Copy link
Member

Timer commented Apr 13, 2020

Please remove target: 'serverless', as it's no longer needed to deploy to ZEIT Now.

ZEIT Now no longer uses the target: 'serverless' option, instead, it auto configures a new target (serverless-trace), which is why you see it work there.

We created the new serverless-trace target to fix this exact set of problems!

@WestonThayer
Copy link
Contributor Author

Awesome, thanks @Timer! I'd initially had target: "serverless" in there while investigating #8685, so it is useful to me to know how to fully emulate the production build locally. From your comment it looks like the experimental-serverless-trace target is it.

Probably not a mainstream use case, but I'd cast my vote for adding experimental-serverless-trace to the docs with a brief explanation.

@WestonThayer
Copy link
Contributor Author

@Timer P.S. is the fact that Zeit NOW is modifying next.config.js during a build documented anywhere? Would cast a vote for this as well — magic is fine, I just need to know about it 😅

@Timer
Copy link
Member

Timer commented Apr 13, 2020

That docs section captures it:

Deployments to ZEIT Now will automatically enable this target.

Maybe it's not clear that this means editing next.config.js on your behalf?


I'm not 100% sold on having users to enable experimental-serverless-trace locally, as the tracing isn't done by Next.js—thus you're not really emulating the final build on ZEIT Now.
The Next.js output for server (the default) and experimental-serverless-trace are going to converge on a "UMD"-like universal format very soon anyway.

target: 'serverless' will only exist for backwards compatibility and manual deployments to AWS Lambda / Azure Functions / Firebase etc.

Also, to be exceptionally clear: custom servers and next start are going to continue to work fine with zero necessary changes under the new universal format.

@Timer
Copy link
Member

Timer commented Apr 13, 2020

Oh, also: setting the serverless target locally used to be more important because serverless would disable your ability to use publicRuntimeConfig / serverRuntimeConfig, however, we added backwards compatibility for these properties—so they're functionally equivalent now (you don't lose anything by switching targets)!

@WestonThayer
Copy link
Contributor Author

Maybe it's not clear that this means editing next.config.js on your behalf?

It is, and that's how I understood it to work back in next@8.1.0. But sounds like the heading it's under ("serverless target") is out of date? Deployments to ZEIT Now use experimental-serverless-trace, not serverless.

I'd be fine with next.config.js's target property entirely disappearing in the future — I can always pull the build output off of ZEIT Now. I just need to know what ZEIT Now is doing so I can debug / find workarounds for issues like #8685.

Right now, it seems like those docs are missing. https://zeit.co/docs/configuration#project/builds clearly shows that builders like @now/next are deprecated, replaced with https://zeit.co/docs/v2/build-step.

But it's confusing that the Next.js build step preset claims to only effect the build command, output directory, and development command:

screenshot of ZEIT Now's build and development settings

But from what you've said, it also adjusts next.config.js. Is there a way I can view the source of the Next.js build step preset? Or does it get a custom runtime?

@Timer
Copy link
Member

Timer commented Apr 14, 2020

Thanks for this excellent feedback. We'll take it into account and rework the docs around this.

Regarding your questions above, deployments to ZEIT Now still use our builders (via zero config). Meaning @now/next still runs the show!

This is the code that forces the target:
https://github.com/zeit/now/blob/master/packages/now-next/src/create-serverless-config.ts

This is the code that wraps the build output for ZEIT Now:
https://github.com/zeit/now/blob/master/packages/now-next/src/index.ts

@WestonThayer
Copy link
Contributor Author

Awesome, y'all continue to be the best.

@GuillaumeBecker
Copy link

I am also facing the same issue. The only difference is I am not using ZEIT Now for hosting and deployments. I use the serverless-next.js pkg since I need to deploy my NextJS project to AWS.

So I can't remove the target: 'serverless' from my next.config.js file. Is there something that can be done ? I am quite stuck on this.

I am using the same version of NextJS and Firebase version as mentioned above. Thanks.

@Timer
Copy link
Member

Timer commented Apr 17, 2020

@GuillaumeBecker unfortunately, this is a fundamental problem with typical serverless deployments (single file bundle) and ultimately why we moved away from the pattern for ZEIT Now. There's no easy solution, but you can try to get serverless-next.js to use the new target.

@GuillaumeBecker
Copy link

@Timer thx for the quick response. I will try to see with serverless-next.js then. When you mentioned to use the new target, do you mean target: 'experimental-serverless-trace' ?

@Timer
Copy link
Member

Timer commented Apr 17, 2020

Correct, experimental-serverless-trace! This target still outputs the entrypoints in the format required for a serverless-type-platform, but requires you ship node_modules alongside it.

@mateusvahl
Copy link

Just create a discussion about that here:
https://github.com/vercel/next.js/discussions/21174

Would be nice to have it on the docs

@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 Jan 28, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants