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

Route level config alt #8863

Merged
merged 10 commits into from
Feb 2, 2023
Merged

Conversation

Rich-Harris
Copy link
Member

@Rich-Harris Rich-Harris commented Feb 2, 2023

Follow-up to #8740 (comment). It's increasingly apparent that builder.createEntries is the wrong API — it tries to make it simple to merge routes into a single function, but the result is anything but.

The approach here is much simpler — just expose the RouteDefinition objects to the builder directly, and let it do whatever it wants.

WIP — need to kick the tyres a bit more before committing

  • validate runtime and other options
  • default to all region for edge functions, not iad1

Copy link
Member

@dummdidumm dummdidumm left a comment

Choose a reason for hiding this comment

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

Yeah I like this much better. I mainly pushed out the previous commit to show the not-good situation we are in with the API. Didn't think of this simple solution though 😃

/** `true` if one or more routes have `export const config = ..` in it */
hasRouteLevelConfig: boolean;
/** An array of dynamic (not prerendered) routes */
routes: RouteDefinition[];
Copy link
Member

Choose a reason for hiding this comment

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

Note for later: The Vercel adapter needs a major version bump because the peer dependency on kit needs a bump so it can expect this API to be present.

Copy link
Member

Choose a reason for hiding this comment

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

Adapter-Auto, too, then. So we also need to update create-svelte to use the latest version.

@Rich-Harris
Copy link
Member Author

it's in a mergeable state so let's carry on in the other branch

@Rich-Harris Rich-Harris merged commit 39f5740 into route-level-config Feb 2, 2023
@Rich-Harris Rich-Harris deleted the route-level-config-alt branch February 2, 2023 22:55
Rich-Harris added a commit that referenced this pull request Feb 6, 2023
* route level config wip

* this seems to work

* proper config

* do shallow merge

* docs

* use config

* default config

* vercel docs

* fix tests

* appease TS

* oops

* more docs

* this feels clunky

* Route level config alt (#8863)

* Revert "this feels clunky"

This reverts commit b3b1b6e.

* alternative approach to route-level config

* remove hasRouteLevelConfig

* handle data requests

* simplify

* fix types

* remove unused code

* fix site

* fix default runtime

* validate region, default to all for edge

* "all" not ["all"]

* changesets

* make defaultConfig a separate option key

* Update documentation/docs/25-build-and-deploy/90-adapter-vercel.md

* implement split: true

* tidy up

* fix site

* Update packages/adapter-vercel/index.d.ts

* tweaks

* get rid of top-level split option

* union type

* handle common case of one fn for everything separately to not pollute json for big apps

* tweak docs a little

* netlify

* make external a config option and simplify adapter options interface

* silence type union error

* use platform defaults

* include everything in builder.routes

* implement ISR

* fix some docs stuff

* clarify multi-region serverless is only for enterprise

* clarify memory stuff

* document ISR

* docs tweaks

* fix site

* add isr in config hash

* bump adapter-auto etc

* bump peerdeps

---------

Co-authored-by: Rich Harris <[email protected]>
Co-authored-by: Rich Harris <[email protected]>
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