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

Pass adapters to config directly? #439

Closed
Rich-Harris opened this issue Mar 4, 2021 · 1 comment · Fixed by #579
Closed

Pass adapters to config directly? #439

Rich-Harris opened this issue Mar 4, 2021 · 1 comment · Fixed by #579

Comments

@Rich-Harris
Copy link
Member

Rich-Harris commented Mar 4, 2021

At the moment you do this:

module.exports = {
  kit: {
    adapter: ['@sveltejs/adapter-node', { whatever: 42 }]
  }
};

It might be nicer to do this...

const node = require('@sveltejs/adapter-node');

module.exports = {
  kit: {
    adapter: node({ whatever: 42 })
  }
};

It would make testing and experimentation and debugging easier, and is just generally preferable to passing a string (or a [string, options] array which is a pattern I've always disliked).

Two downsides: it means adapters load eagerly rather than lazily (i.e. they get loaded in dev, even though they don't do anything) and it would mean adapters needed to expose CommonJS rather than ESM, unless we did this...

import node from '@sveltejs/adapter-node';

export default {
  kit: {
    adapter: node({ whatever: 42 });
  }
};

...which we can't at the moment because language tools requires the config to be CommonJS.

@Rich-Harris
Copy link
Member Author

Another benefit of importing the adapters: the options can be typed. Proposed shape:

type Adapter = {
  adapt: (builder: Builder) => void | Promise<void>
};

Then an adapter author can do this (as long as @sveltejs/kit is installed as a dev dependency):

/**
 * @param {{ whatever: number }} options
 * /
export default async function adapter(options) {
  /** @type {import('@sveltejs/kit').Adapter */
  return {
    adapt: builder => {
      // `builder` is typed
    }
  };
}

Note that this is a different signature to the adapters at the moment, which expose a (builder, options) => void function as default. Would be nice to resolve this before the public beta, though @dummdidumm indicated that it would tricky to have an ESM config file (#349 (comment)) so we might be stuck shipping CJS for the time being.

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 a pull request may close this issue.

1 participant