Skip to content

Filter out node_modules #346

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 3 commits into from
Jun 30, 2025
Merged

Filter out node_modules #346

merged 3 commits into from
Jun 30, 2025

Conversation

@Yuangwang
Copy link
Collaborator

@Yuangwang Yuangwang commented Jun 26, 2025

Filters out all directories other than .next/standalone, in practice this will pretty much only delete the extra node_modules in the root directory, improving build speeds

jamesdaniels
jamesdaniels reviewed Jun 27, 2025
View reviewed changes
packages/@apphosting/adapter-nextjs/src/utils.ts Outdated
},
outputFiles: {
serverApp: {
include: [".next/standalone"],
Copy link
Collaborator

@jamesdaniels jamesdaniels Jun 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm assuming we're already moving in .next/static and public.

Also, let's only do this for developers manually specifying output: "standalone" for now, so we aren't breaking anyone. I see if I can wrap the draft MSA today.

Copy link
Collaborator Author

@Yuangwang Yuangwang Jun 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We’re actually moving everything else still, the only thing we’re removing with this change is the root dir’s node_modules.

Isn’t that extra in all cases so this shouldn’t break anyone?

annajowang
annajowang approved these changes Jun 30, 2025
View reviewed changes
Copy link
Collaborator

@annajowang annajowang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

one documentation nit

@Yuangwang Yuangwang merged commit 96c420b into main Jun 30, 2025
11 checks passed
@mehranabi
Copy link

mehranabi commented Jul 4, 2025

It breaks monorepo setup.
We have our nextjs app at "apps/web" and our build output is at "apps/web/.next/standalone" and with this change our build goes through but the output is not copied to the final image.

@mehranabi mehranabi mentioned this pull request Jul 4, 2025
Open
mathu97 added a commit that referenced this pull request Jul 7, 2025
@mathu97
Revert "Filter out node_modules (#346)"
1f25235 
This reverts commit 96c420b.
@mathu97 mathu97 mentioned this pull request Jul 7, 2025
Merged
mathu97 added a commit that referenced this pull request Jul 7, 2025
@mathu97
Revert "Filter out node_modules" (#350)
4675e3e 
* Revert "Filter out node_modules (#346)"

This reverts commit 96c420b.

* bump nextjs adapter version to 14.0.16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jamesdaniels jamesdaniels jamesdaniels left review comments

@annajowang annajowang annajowang approved these changes

@sjjj986 sjjj986 sjjj986 approved these changes

@blidd-google blidd-google Awaiting requested review from blidd-google blidd-google is a code owner

@taeold taeold Awaiting requested review from taeold taeold is a code owner

@leoortizz leoortizz Awaiting requested review from leoortizz leoortizz is a code owner

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@Yuangwang @mehranabi @jamesdaniels @annajowang @sjjj986