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

Add ttl option to netlify adapter for builders #6219

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 35 additions & 22 deletions packages/integrations/netlify/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ Learn how to deploy your Astro site in our [Netlify deployment guide](https://do
- <strong>[Contributing](#contributing)</strong>
- <strong>[Changelog](#changelog)</strong>


## Why Astro Netlify

If you're using Astro as a static site builder—its behavior out of the box—you don't need an adapter.
Expand All @@ -22,7 +21,6 @@ If you wish to [use server-side rendering (SSR)](https://docs.astro.build/en/gui

[Netlify](https://www.netlify.com/) is a deployment platform that allows you to host your site by connecting directly to your GitHub repository. This adapter enhances the Astro build process to prepare your project for deployment through Netlify.


## Installation

Add the Netlify adapter to enable SSR in your Astro project with the following `astro add` command. This will install the adapter and make the appropriate changes to your `astro.config.mjs` file in one step.
Expand All @@ -40,22 +38,22 @@ If you prefer to install the adapter manually instead, complete the following tw

1. Install the Netlify adapter to your project’s dependencies using your preferred package manager. If you’re using npm or aren’t sure, run this in the terminal:

```bash
npm install @astrojs/netlify
```
```bash
npm install @astrojs/netlify
```

1. Add two new lines to your `astro.config.mjs` project configuration file.

```js ins={3, 6-7}
// astro.config.mjs
import { defineConfig } from 'astro/config';
import netlify from '@astrojs/netlify/functions';
```js ins={3, 6-7}
// astro.config.mjs
import { defineConfig } from 'astro/config';
import netlify from '@astrojs/netlify/functions';

export default defineConfig({
output: 'server',
adapter: netlify(),
});
```
export default defineConfig({
output: 'server',
adapter: netlify(),
});
```

### Edge Functions

Expand Down Expand Up @@ -88,7 +86,6 @@ netlify deploy --build

The [Netlify Blog post on Astro](https://www.netlify.com/blog/how-to-deploy-astro/) and the [Netlify Documentation](https://docs.netlify.com/integrations/frameworks/astro/) provide more information on how to use this integration to deploy to Netlify.


## Configuration

To configure this adapter, pass an object to the `netlify()` function call in `astro.config.mjs` - there's only one possible configuration option:
Expand All @@ -105,8 +102,8 @@ import netlify from '@astrojs/netlify/functions';
export default defineConfig({
output: 'server',
adapter: netlify({
dist: new URL('./dist/', import.meta.url)
})
dist: new URL('./dist/', import.meta.url),
}),
});
```

Expand All @@ -129,14 +126,31 @@ import netlify from '@astrojs/netlify/functions';
export default defineConfig({
output: 'server',
adapter: netlify({
builders: true
builders: true,
}),
});
```

On-demand Builders are only available with the `@astrojs/netlify/functions` adapter and are not compatible with Edge Functions.
On-demand Builders optionally let you return a [TTL](https://docs.netlify.com/configure-builds/on-demand-builders/#time-to-live-ttl) (time-to-live) value for the cached page. The astro adapter allows you to set a default TTL value for all pages, or a custom TTL value for specific pages via a function call.
Copy link
Member

Choose a reason for hiding this comment

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

A few wording suggestions:

  1. I believe that "Astro adapter" is redundant, considering the context it's clear we're talking about the current adapter, so I would remove "Astro".
  2. We're repeating "TTL value" a few times in this sentence, what do you think of simplifying it a bit?
Suggested change
On-demand Builders optionally let you return a [TTL](https://docs.netlify.com/configure-builds/on-demand-builders/#time-to-live-ttl) (time-to-live) value for the cached page. The astro adapter allows you to set a default TTL value for all pages, or a custom TTL value for specific pages via a function call.
On-demand Builders optionally let you return a [TTL](https://docs.netlify.com/configure-builds/on-demand-builders/#time-to-live-ttl) (time-to-live) value for the cached page. The adapter allows you to set a default TTL value for all pages or a custom one for specific pages via a function call.


```js
// astro.config.mjs
import { defineConfig } from 'astro/config';
import netlify from '@astrojs/netlify/functions';

export default defineConfig({
output: 'server',
adapter: netlify({
builders: {
ttl(event) {
return event.path.startsWith('/blog') ? 3600 : undefined;
},
},
}),
});
```

On-demand Builders are only available with the `@astrojs/netlify/functions` adapter and are not compatible with Edge Functions.
Copy link
Member

Choose a reason for hiding this comment

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

The sentence above also starts with "On-demand Builders" and I think we should call a bit more attention to it, so I changed it a bit adding "Note that" so people notice this part more!

Suggested change
On-demand Builders are only available with the `@astrojs/netlify/functions` adapter and are not compatible with Edge Functions.
Note that On-demand Builders are only available with the `@astrojs/netlify/functions` adapter and are not compatible with Edge Functions.

Copy link
Author

Choose a reason for hiding this comment

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

@Yan-Thomas Great suggestions, thanks 🙏


### binaryMediaTypes

Expand All @@ -158,8 +172,8 @@ export function get() {
return new Response(buffer, {
status: 200,
headers: {
'content-type': 'image/jpeg'
}
'content-type': 'image/jpeg',
},
});
}
```
Expand All @@ -185,4 +199,3 @@ This package is maintained by Astro's Core team. You're welcome to submit an iss
See [CHANGELOG.md](CHANGELOG.md) for a history of changes to this integration.

[astro-integration]: https://docs.astro.build/en/guides/integrations-guide/

7 changes: 6 additions & 1 deletion packages/integrations/netlify/src/integration-functions.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import type { AstroAdapter, AstroConfig, AstroIntegration } from 'astro';
import type { HandlerEvent } from '@netlify/functions';
import type { Args } from './netlify-functions.js';
import { createRedirects } from './shared.js';

Expand All @@ -13,7 +14,11 @@ export function getAdapter(args: Args = {}): AstroAdapter {

interface NetlifyFunctionsOptions {
dist?: URL;
builders?: boolean;
builders?:
| boolean
| {
ttl?: number | ((event: HandlerEvent) => number | undefined);
};
binaryMediaTypes?: string[];
}

Expand Down
17 changes: 15 additions & 2 deletions packages/integrations/netlify/src/netlify-functions.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { polyfill } from '@astrojs/webapi';
import { builder, Handler } from '@netlify/functions';
import { builder, Handler, HandlerEvent } from '@netlify/functions';
import { SSRManifest } from 'astro';
import { App } from 'astro/app';

Expand All @@ -8,7 +8,11 @@ polyfill(globalThis, {
});

export interface Args {
builders?: boolean;
builders?:
| boolean
| {
ttl?: number | ((event: HandlerEvent) => number | undefined);
Copy link
Contributor

Choose a reason for hiding this comment

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

Unfortunately this kind of API won't work. Args gets serialized and included in the build. Your astro.config.mjs does not get bundled with the build. So only plain objects/primitives can be passed into the actual integration. Functions won't work.

How we usually fix this is by instead having the code be defined in a separate module and then building that module as part of the build. Something: builders: 'path/to/implementation.ts'. It's not as nice of an API but it prevents us from needing to build/bundle the config file

Copy link
Author

Choose a reason for hiding this comment

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

@matthewp Oh wow, I wasn't aware of that. Does this only apply to adapter configurations? After all we can use Vite plugins in astro.config that again are (partly) functions: https://docs.astro.build/en/reference/configuration-reference/#vite – on the other hand I can't find any example of passing a path to .ts or .js file. If you know of any other usage, that would be very helpful indeed.

Copy link
Contributor

Choose a reason for hiding this comment

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

We don't build the config, so any functions that need to exist in production cannot be provided via a config option.

As for examples, all of the renderers work this way, see for example:

return {
name: '@astrojs/vue',
clientEntrypoint: '@astrojs/vue/client.js',
serverEntrypoint: '@astrojs/vue/server.js',
};

I'd suggest a similar pattern if you want to allow the user to customize runtime values.

};
binaryMediaTypes?: string[];
}

Expand Down Expand Up @@ -132,6 +136,15 @@ export const createExports = (manifest: SSRManifest, args: Args) => {
}
}

if (typeof builders === 'object' && 'ttl' in builders) {
const ttlSetting = builders.ttl;
if (typeof ttlSetting === 'number') {
fnResponse.ttl = ttlSetting;
} else if (typeof ttlSetting === 'function') {
fnResponse.ttl = ttlSetting(event);
}
}

// Apply cookies set via Astro.cookies.set/delete
if (app.setCookieHeaders) {
const setCookieHeaders = Array.from(app.setCookieHeaders(response));
Expand Down