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

feat(vite-plugin-nitro): add internal deployment support for Netlify #1202

Closed
wants to merge 2 commits into from

Conversation

pieh
Copy link

@pieh pieh commented Jul 3, 2024

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Documentation content changes
  • Other... Please describe:

Which package are you modifying?

  • vite-plugin-angular
  • vite-plugin-nitro
  • vitest-angular
  • astro-angular
  • create-analog
  • router
  • platform
  • content
  • nx-plugin
  • trpc

What is the current behavior?

Users deploying to Netlify need to additionally configure location of produced serverless functions

Closes #

What is the new behavior?

Serverless functions are generated at location that Netlify platform automatically handle removing the need to specify ~custom serverless function directory when configuring deployments to Netlify.

Additional note is that hopefully soon at least non-nx variant would require zero configuration. We recently added Analog to frameworks we automatically detect ( netlify/build#5751 ) which soon will suggest appropiate settings when creating Analog site in Netlify web ui and there is work in-progress to use same detection automatically configure deployment when using Netlify CLI ( netlify/cli#6729 )

Does this PR introduce a breaking change?

  • Yes
  • No

Explanation: users defining Netlify Functions directory with current instructions will still be able to deploy successfully because Netlify always look for functions in adjusted directory, so even if we point functions directory to one that no longer exist (or at least doesn't have Analog function) it will work correctly:

Packaging Functions from .netlify/functions-internal directory:
 - server/server.mjs
No Functions were found in dist/analog directory

Other information

[optional] What gif best describes this PR or how it makes you feel?

Copy link

netlify bot commented Jul 3, 2024

Deploy Preview for analog-blog ready!

Name Link
🔨 Latest commit 2dade9b
🔍 Latest deploy log https://app.netlify.com/sites/analog-blog/deploys/66853fb1fd20ea0008014cde
😎 Deploy Preview https://deploy-preview-1202--analog-blog.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link

netlify bot commented Jul 3, 2024

Deploy Preview for analog-app ready!

Name Link
🔨 Latest commit 2dade9b
🔍 Latest deploy log https://app.netlify.com/sites/analog-app/deploys/66853fb17e9c7900089846a6
😎 Deploy Preview https://deploy-preview-1202--analog-app.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link

netlify bot commented Jul 3, 2024

Deploy Preview for analog-docs ready!

Name Link
🔨 Latest commit 2dade9b
🔍 Latest deploy log https://app.netlify.com/sites/analog-docs/deploys/66853fb1cc820a0008834fe3
😎 Deploy Preview https://deploy-preview-1202--analog-docs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link

netlify bot commented Jul 3, 2024

Deploy Preview for analog-ng-app ready!

Name Link
🔨 Latest commit 2dade9b
🔍 Latest deploy log https://app.netlify.com/sites/analog-ng-app/deploys/66853fb1e4651b00086362af
😎 Deploy Preview https://deploy-preview-1202--analog-ng-app.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@brandonroberts
Copy link
Member

Thanks @pieh! Can this be zero-config for static assets also?

@pieh
Copy link
Author

pieh commented Jul 3, 2024

Can this be zero-config for static assets also?

With netlify/build#5751 we will soon automatically pre-fill configuration when creating a new site on Netlify web app (once that change actually end up in production), so this flow will be ~somewhat zero conf for static assets soon.

For CLI usage (netlify deploy --build) for now users will still need to define deploy settings in netlify.toml configuration file, but this no longer will be needed when netlify/cli#6729 is finished, so eventually it will be fully zero conf, just not yet.

Netlify doesn't have equivalent to something like Vercel's output api for static assets (https://github.com/analogjs/analog/blob/beta/packages/vite-plugin-nitro/src/lib/vite-plugin-nitro.ts#L354-L356) where there is always specific directory from which static assets are uploaded - Netlify relies on "publish directory" for those things, so instead of frameworks putting those files in deployment specific directory, either user or our framework detection tell us where framework outputs static assets

With that said, I think something is not exactly right with either this change or configuration of test Netlify sites/apps because preview deploy for analog-app 404s for API route ( https://deploy-preview-1202--analog-app.netlify.app/api/v1/products ) which wasn't happening for previous pull requests ( https://deploy-preview-1201--analog-app.netlify.app/api/v1/products ), so this probably should not be merged yet until this is figured out what's wrong.

I did attempt to deploy this app on my side of things locally with following configuration (so functions dir is not set, relying on the change I did):

# netlify.toml
[build]
command = "nx build analog-app"
publish = "dist/apps/analog-app/analog/public"

and I did get working deploy ( https://66855a09ed43201e349b11f4--analog-app-no-functions-defined-test.netlify.app/api/v1/products ), so some debugging is needed why deploy preview on this PR is not actually working correctly

@brandonroberts
Copy link
Member

Thanks for getting better Analog support merged into Netlify!

Ok cool. It could be my manual deployment settings that are conflicting.

image

@pieh
Copy link
Author

pieh commented Jul 4, 2024

I think I have clue given the site settings - the "Package path" is not set here while mine was equivalent to apps/analog-app and likely the function was generated in apps/analog-app/.netlify/functions-internal, while Netlify would look for /.netlify/functions-internal when "Package path" is not set.

So while I do think it would be beneficial for site to specify "correct" package path, any change in this PR should NOT break the sites with currently working setting, so I'll look if I can figure out setup that would just work for both scenarios with Package Path set and not set.

I don't have much Nitro knowledge and the dir I set here is just straight copied from Nitro's Netlify preset ( https://github.com/unjs/nitro/blob/63526f24b03e5e5de65934c697c92dd00f1e1ab1/src/presets/netlify/preset.ts#L20 ), so this change likely "inherited" the same problem from the original setting

...nitroConfig,
output: {
...nitroConfig?.output,
dir: '{{ rootDir }}/.netlify/functions-internal',
Copy link
Member

Choose a reason for hiding this comment

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

I think what we need to do here is set the serverDir. The dir sets the output for everything else, including the publicDir and .nitro build directory.

Suggested change
dir: '{{ rootDir }}/.netlify/functions-internal',
serverDir: '{{ rootDir }}/.netlify/functions-internal',

@brandonroberts
Copy link
Member

Yea, I don't usually use the package path because even in a monorepo setup like Nx, I build from the root and use the project path to build where the output goes in the dist folder, which I think is more common.

@brandonroberts
Copy link
Member

@pieh were you able to find anything else on this?

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 this pull request may close these issues.

2 participants