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] use Netlify internal functions directory for generated functions #2113

Merged
merged 8 commits into from
Aug 18, 2021
Merged
Show file tree
Hide file tree
Changes from 7 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
5 changes: 5 additions & 0 deletions .changeset/brave-clouds-protect.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@sveltejs/adapter-netlify': patch
---

Deploy generated Netlify entrypoint to the internal functions directory. This allows it to co-exist with other Netlify functions.
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -11,3 +11,4 @@ yarn.lock
.svelte-kit
.cloudflare
.pnpm-debug.log
.netlify
2 changes: 1 addition & 1 deletion examples/hn.svelte.dev/.gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,4 @@
node_modules
/.svelte
/build
/functions
/.netlify/functions*
benmccann marked this conversation as resolved.
Show resolved Hide resolved
5 changes: 3 additions & 2 deletions examples/hn.svelte.dev/.netlify/netlify-plugin-pnpm/index.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
export default {
module.exports = {
benmccann marked this conversation as resolved.
Show resolved Hide resolved
onPreBuild: async ({ utils: { build, run } }) => {
try {
await run.command("npm install -g pnpm")
await run.command("pnpm install")
await run.command("pnpm -w install")
await run.command("pnpm -w build")
} catch (error) {
return build.failBuild(error)
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
{"type":"commonjs"}
3 changes: 1 addition & 2 deletions examples/hn.svelte.dev/netlify.toml
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
[build]
command = "npm run build"
publish = ".svelte-kit/netlify/build/"
functions = ".svelte-kit/netlify/functions/"
publish = "build"

[build.environment]
NPM_FLAGS="--prefix=/dev/null"
Expand Down
24 changes: 20 additions & 4 deletions packages/adapter-netlify/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -27,15 +27,16 @@ export default {
};
```

Then, make sure you have a [netlify.toml](https://docs.netlify.com/configure-builds/file-based-configuration) file in the project root. This will determine where to write static assets and functions to based on the `build.publish` and `build.functions` settings, as per this sample configuration:
Then, make sure you have a [netlify.toml](https://docs.netlify.com/configure-builds/file-based-configuration) file in the project root. This will determine where to write static assets based on the `build.publish` settings, as per this sample configuration:

```toml
[build]
command = "npm run build"
publish = "build/publish/"
functions = "build/functions/"
Copy link
Member

Choose a reason for hiding this comment

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

I don't know if we can just remove this. Where will it put functions then? We put this here on purpose so that all output would show up under build and be automatically gitignored without the user having to edit their .gitignore at all. It would be nice if that continued to work for people who are not using the Netlify CLI

Copy link
Member

Choose a reason for hiding this comment

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

Or at least if we're going to remove this line, should we also remove the publish line so that ends up under .netlify/publish or something? It seems weird to treat these inconsistently.

Perhaps what we could do is maintain the old behavior and use the directories in these files if they are present and if they are not then we use the directory you're specifying as a default

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The functions will be in .netlify/functions-internal. The functions directory isn't needed anymore, because we're using the internal functions directory instead, which is hard-coded. The idea is that the functions value is for user functions, while .netlify/functions-internal is for auto-generated functions. If we remove the publish dir it will default to static because that's the value specified for SvelteKit. Should that be changed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If they user isn't using the Netlify CLI then presumably this won't be an issue anyway, as it wouldn't be using the Netlify preset when building locally as the auto-detection won't trigger it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Though I suppose the user could have manually specified the preset.

Copy link
Member

Choose a reason for hiding this comment

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

Ok, thanks for clarifying and being patient as I come up-to-speed on Netlify stuff. I'm on board with your plan. Lets always output to .netlify/functions-internal

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK, so skip the CLI detection entirely?

Copy link
Member

Choose a reason for hiding this comment

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

What would be the benefit of CLI detection? If we're not using the Netlify CLI, I'm assuming that any code .netlify/functions-internal will still be used?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Correct. I'll update the PR.

Copy link
Contributor Author

@ascorbic ascorbic Aug 18, 2021

Choose a reason for hiding this comment

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

OK, I've updated the PR as follows:

  • Always write out the generated functions to .netlify/functions-internal
  • Read "publish" from netlify.toml, but if it's missing then default to "build"
  • Add more warnings and checks, such as ensuring that publish isn't set to the site root (that would delete the site!)
  • Updated the README

One thing I'd like to suggest for a follow-up PR is that cwd is passed to adapter functions here. That would allow us to ensure that we resolve all the paths correctly inside adapters. I may be misunderstanding it as I've not tested it, but as it stands I'm not sure if these will all work if cwd isn't process.cwd() This would likely apply to most adapters, as they all seem to assume paths are relative to process.cwd().

publish = "build"
```

If the `netlify.toml` file or the `build.publish` value is missing, a default value of `"build"` will be used. Note that if you have set the publish directory in the Netlify UI to something else then you will need to set it in `netlify.toml` too, or use the default value of `"build"`.

## Netlify alternatives to SvelteKit functionality

You may build your app using functionality provided directly by SvelteKit without relying on any Netlify functionality. Using the SvelteKit versions of these features will allow them to be used in dev mode, tested with integration tests, and to work with other adapters should you ever decide to switch away from Netlify. However, in some scenarios you may find it beneficial to use the Netlify versions of these features. One example would be if you're migrating an app that's already hosted on Netlify to SvelteKit.
Expand All @@ -53,6 +54,20 @@ During compilation a required "catch all" redirect rule is automatically appende
2. Netlify's build bot parses your HTML files at deploy time, which means your form must be [prerendered](https://kit.svelte.dev/docs#ssr-and-javascript-prerender) as HTML. You can either add `export const prerender = true` to your `contact.svelte` to prerender just that page or set the `kit.prerender.force: true` option to prerender all pages.
3. If your Netlify form has a [custom success message](https://docs.netlify.com/forms/setup/#success-messages) like `<form netlify ... action="/success">` then ensure the corresponding `/routes/success.svelte` exists and is prerendered.

### Using Netlify Functions

[Netlify Functions](https://docs.netlify.com/functions/overview/) can be used alongside your SvelteKit routes. If you would like to add them to your site, you should create a directory for them and add the configuration to your `netlify.toml` file. For example:

```toml
[build]
command = "npm run build"
publish = "build"

[functions]
directory = "functions"
node_bundler = "esbuild"
```

## Advanced Configuration

### esbuild
Expand All @@ -77,7 +92,8 @@ The default options for this version are as follows:
```js
{
entryPoints: ['.svelte-kit/netlify/entry.js'],
outfile: `pathToFunctionsFolder/render/index.js`,
// This is Netlify's internal functions directory, not the one for user functions.
outfile: '.netlify/functions-internal/__render.js',
bundle: true,
inject: ['pathTo/shims.js'],
platform: 'node'
Expand Down
49 changes: 30 additions & 19 deletions packages/adapter-netlify/index.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { appendFileSync, existsSync, readFileSync, writeFileSync } from 'fs';
import { join } from 'path';
import { join, resolve } from 'path';
import { fileURLToPath } from 'url';
import esbuild from 'esbuild';
import toml from '@iarna/toml';
Expand All @@ -19,10 +19,12 @@ export default function (options) {
name: '@sveltejs/adapter-netlify',

async adapt({ utils }) {
const { publish, functions } = validate_config().build;
// "build" is the default publish directory when Netlify detects SvelteKit
const publish = get_publish_directory(utils) || 'build';

utils.log.minor(`Publishing to "${publish}"`);

utils.rimraf(publish);
utils.rimraf(functions);

const files = fileURLToPath(new URL('./files', import.meta.url));

Expand All @@ -32,7 +34,9 @@ export default function (options) {
/** @type {BuildOptions} */
const defaultOptions = {
entryPoints: ['.svelte-kit/netlify/entry.js'],
outfile: join(functions, 'render/index.js'),
// Any functions in ".netlify/functions-internal" are bundled in addition to user-defined Netlify functions.
ascorbic marked this conversation as resolved.
Show resolved Hide resolved
// See https://github.com/netlify/build/pull/3213 for more details
outfile: '.netlify/functions-internal/__render.js',
benmccann marked this conversation as resolved.
Show resolved Hide resolved
bundle: true,
inject: [join(files, 'shims.js')],
platform: 'node'
Expand All @@ -43,9 +47,9 @@ export default function (options) {

await esbuild.build(buildOptions);

writeFileSync(join(functions, 'package.json'), JSON.stringify({ type: 'commonjs' }));
writeFileSync(join('.netlify', 'package.json'), JSON.stringify({ type: 'commonjs' }));

utils.log.info('Prerendering static pages...');
utils.log.minor('Prerendering static pages...');
await utils.prerender({
dest: publish
});
Expand All @@ -55,16 +59,21 @@ export default function (options) {
utils.copy_client_files(publish);

utils.log.minor('Writing redirects...');
utils.copy('_redirects', `${publish}/_redirects`);
appendFileSync(`${publish}/_redirects`, '\n\n/* /.netlify/functions/render 200');

const redirectPath = join(publish, '_redirects');
utils.copy('_redirects', redirectPath);
appendFileSync(redirectPath, '\n\n/* /.netlify/functions/__render 200');
benmccann marked this conversation as resolved.
Show resolved Hide resolved
}
};

return adapter;
}

function validate_config() {
/**
* @param {import('@sveltejs/kit').AdapterUtils} utils
**/
function get_publish_directory(utils) {
if (existsSync('netlify.toml')) {
/** @type {{ build?: { publish?: string }} & toml.JsonMap } */
let netlify_config;

try {
Expand All @@ -74,23 +83,25 @@ function validate_config() {
throw err;
}

if (!netlify_config.build || !netlify_config.build.publish || !netlify_config.build.functions) {
throw new Error(
'You must specify build.publish and build.functions in netlify.toml. Consult https://github.com/sveltejs/kit/tree/master/packages/adapter-netlify#configuration'
);
if (!netlify_config.build || !netlify_config.build.publish) {
utils.log.warn('No publish directory specified in netlify.toml, using default');
return;
}

if (netlify_config.redirects) {
throw new Error(
"Redirects are not supported in netlify.toml. Use _redirects instead. For more details consult the readme's troubleshooting section."
);
}

return netlify_config;
if (resolve(netlify_config.build.publish) === process.cwd()) {
throw new Error(
'The publish directory cannot be set to the site root. Please change it to "build" in netlify.toml.'
benmccann marked this conversation as resolved.
Show resolved Hide resolved
);
}
return netlify_config.build.publish;
}

// TODO offer to create one?
throw new Error(
'Missing a netlify.toml file. Consult https://github.com/sveltejs/kit/tree/master/packages/adapter-netlify#configuration'
utils.log.warn(
'No netlify.toml found. Using default publish directory. Consult https://github.com/sveltejs/kit/tree/master/packages/adapter-netlify#configuration for more details '
);
}
3 changes: 1 addition & 2 deletions packages/create-svelte/templates/default/netlify.toml
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
[build]
command = "npm run build"
publish = ".svelte-kit/netlify/build/"
functions = ".svelte-kit/netlify/functions/"
publish = "build"