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

Number or resources > 200 #17

Closed
romainquellec opened this issue Mar 20, 2019 · 19 comments
Closed

Number or resources > 200 #17

romainquellec opened this issue Mar 20, 2019 · 19 comments
Labels
enhancement New feature or request

Comments

@romainquellec
Copy link

Finally deploying my project en AWS, I've got this :

The CloudFormation template is invalid: Template format error: Number of resources, 226, is greater than maximum allowed, 200

I dont know this error. FWI, I've got 37 pages.

@romainquellec
Copy link
Author

romainquellec commented Mar 20, 2019

I'm looking at serverless-nested-stack, but it seems incompatible with serverless-nextjs-plugin
Looking at serverless-plugin-split-stacks right now.

@romainquellec
Copy link
Author

Trying some options with serverless-plugin-split-stacks, but the lambdas are not deployed.
Maybe, I'm using the plugins wrong.

@romainquellec
Copy link
Author

romainquellec commented Mar 25, 2019

IMO this is the most important issue right now, as 30 is not a big number for pages.

@danielcondemarin
Copy link
Contributor

@romainquellec It's definitely on my radar.
Seems like it might be addressed in the serverless framework itself: serverless/serverless#3411

If you don't need function versioning you can switch it off:

# serverless.yml
provider:
  versionFunctions: false

That's 37 less resources.

Also, I will look into why serverless-plugin-split-stacks or nested-stacks doesn't work with the plugin.

@danielcondemarin danielcondemarin added the enhancement New feature or request label Apr 6, 2019
@Vadorequest
Copy link

Doesn't seem like the Serverless framework will tackle the issue anytime soon, I'm not really confident about that. (issue opened in 2017, and still not sure about the right approach, seems that AWS could provide a solution but no ETA either)

Just thinking out of the box, maybe for this kind of need the serverless approach isn't the right one, and a "one route to handle them all" should be considered? Like what we used to do before Next v8, basically using an Express app on top of Next. That's how I'd go if I was in the hurry of a fix right now, I don't see other alternatives.

@romainquellec
Copy link
Author

romainquellec commented May 6, 2019

@Vadorequest I dont think this plugin should go on this "one route to handle them all" because you can do that pretty easily by yourself with a single lambda with next build.
Imo, there is no added value to this plugin for that.

(https://github.com/romainquellec/boilerplate/tree/e72c9a162507519fe8667b8c7d6b3c7b11e59ab5)

@SimonStiph
Copy link

I disagree w/ Romain. In my experience, the one-function solution solves this issue very well in my production Serverless applications. IMHO, I would question what benefit that function splitting is going to provide at all.

@romainquellec
Copy link
Author

In my experience, the one-function solution solves this issue very well in my production Serverless applications. IMHO, I would question what benefit that function splitting is going to provide at all.

In my previous comment, I'm not saying its bad to have only one function (depending of the case). I'm saying this plugin doesn't provide any advantages compared to a simple custom handle. (something like this : https://github.com/romainquellec/boilerplate/blob/e72c9a162507519fe8667b8c7d6b3c7b11e59ab5/server/lambda.ts)

To respond on that :

I would question what benefit that function splitting is going to provide at all.

Cold start
Dynamically build routes
Compatible with next serverless vision

Furthermore using express for the functions will slow them down / increase bundle size.
Also bundling everything into one function means that say you have 10 pages, you'll have 10 versions of React into the bundle. Same for every component etc.

@romainquellec
Copy link
Author

Maybe we should add a section explaining this design as it's a recurrent topic here and also on next.js depot. @danielcondemarin

@SimonStiph
Copy link

Cold start

The one-function method means every viewed page keeps the entire site warm. This is actually a pro for the one-function method (though I recommend using CloudWatch Rules instead anyways.)

Furthermore using express for the functions will slow them down / increase bundle size.

Next.js already bundles Express. It's built on top of it. And since there's no HTTP, Express.js is simply used as a wrapper. You should research how it works; it's very intuitive.

Also bundling everything into one function means that say you have 10 pages, you'll have 10 versions of React into the bundle.

Node.js only has one "node_modules" folder, and all modules are cached after the first import.

Maybe we should add a section explaining this design as it's a recurrent topic here and also on next.js depot.

I agree with that entirely. People are sitting on a ticking time bomb if at some point they can't continue working because of the CloudFormation limit.

@romainquellec
Copy link
Author

romainquellec commented Jun 7, 2019

You should argue with @timneutkens on next.js depot or spectrum as I'm using mainly his arguments because I'm lazy!
At the end : vercel/next.js#6685

The one-function method means every viewed page keeps the entire site warm. This is actually a pro for the one-function method (though I recommend using CloudWatch Rules instead anyways.)

No, because when the function is small, warming is not an issue at all.

Next.js already bundles Express. It's built on top of it. And since there's no HTTP, Express.js is simply used as a wrapper. You should research how it works; it's very intuitive.

https://zeit.co/blog/serverless-express-js-lambdas-with-now-2
"Since each route works with its own instance of Express" => You add an express server for your only route like a middleware, right ?

Node.js only has one "node_modules" folder, and all modules are cached after the first import.

When you build your project with next, you have an artifact per page. You have no node_modules.

Maybe we should add a section explaining this design as it's a recurrent topic here and also on next.js depot.

Finally, we agree :)

Making all theses efforts with nextjs to have independants pages/lambdas to finally have a single artifact, where is the point ? If you don't like this way of doing, you should argue with the next.js team, not really here.

@SimonStiph
Copy link

We disagree on all of that except the documentation update :) But regardless, my take is that this plugin is useful, but is flawed in that hitting the CloudFormation limit is a major showstopper. Before I was going to use this plugin, I purposefully looked for this exact issue since I'm very familiar with AWS and knew of the hard limit. Other developers using this plugin will find out the hard way.

If the identity and usefulness of this plugin are based around splitting up the functions, then by all means. I would still update the docs. to make devs. aware of this limit at least.

@timneutkens
Copy link

timneutkens commented Jun 8, 2019

Next.js already bundles Express. It's built on top of it. And since there's no HTTP, Express.js is simply used as a wrapper. You should research how it works; it's very intuitive.

Next.js does not use express, especially in the serverless target. There is a custom server API that is not compatible with serverless.

Node.js only has one "node_modules" folder, and all modules are cached after the first import.

Anything over 5mb affects cold boot. Check this talk specifically: https://brianleroux.github.io/serverless-seattle-2019/#/15

Hence why Next.js bundles every page into the smallest possible lambda function. The base size is 42Kb gzipped currently. The function can be independently executed from any dependencies (it's standalone).

This has many other benefits though. You get per-route scaling, faster execution etc.

@SimonStiph
Copy link

@timneutkens

Next.js does not use express

Actually, that is correct. It has Express.js listed as a dependency which made me assume that it is based on it, but going through the code I found that it uses Express.js for the bundled examples.

Check this talk specifically: https://brianleroux.github.io/serverless-seattle-2019/#/15

There are problems with your source. It states that pinging your Lambdas will not affect cold starts, and that contradicts what Amazon's Chris Munns statement about keeping them warm. But there is also good information in those slides, I'm not dogging it.

Anything over 5mb affects cold boot.

I've seen evidence in my own projects that supports that theory.

@danielcondemarin
Copy link
Contributor

danielcondemarin commented Jun 11, 2019

We disagree on all of that except the documentation update :) But regardless, my take is that this plugin is useful, but is flawed in that hitting the CloudFormation limit is a major showstopper. Before I was going to use this plugin, I purposefully looked for this exact issue since I'm very familiar with AWS and knew of the hard limit. Other developers using this plugin will find out the hard way.

I will document the CF limitation. Also, I'm currently looking into how the plugin could be ported to a serverless-component, which for the most part uses plain API calls rather than CF.

@romainquellec
Copy link
Author

Any news on serverless-component ?

@danielcondemarin
Copy link
Contributor

Any news on serverless-component ?

I'm currently trying to get v2.0 released first, but for that I need to finish CloudFront support (#95)

After that's sorted will look in more detail how to port to a serverless-component.

@danielcondemarin
Copy link
Contributor

@romainquellec @Vadorequest @SimonStiph I've now released the first version of the nextjs component which addresses this issue: https://github.com/danielcondemarin/serverless-next.js/tree/master/packages/serverless-nextjs-component ⚡️
Please check it out and feedback, that would be very appreciated 🙏

@Vadorequest
Copy link

Very interesting. I may try it very soon for a POC! Nice work!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
5 participants