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

Unbundling #5853

Merged
merged 7 commits into from
Aug 8, 2022
Merged

Unbundling #5853

merged 7 commits into from
Aug 8, 2022

Conversation

Rich-Harris
Copy link
Member

@Rich-Harris Rich-Harris commented Aug 8, 2022

This is part of #5778, but it's a shift in strategy that's separate from #5748 so I figured I'd make this PR as a quick detour to explain it.

Currently, @sveltejs/kit is bundled. Imports like @sveltejs/kit/vite and @sveltejs/kit/node are bundled in dist, while the modules behind the $app alias are bundled in assets, and copied over to .svelte-kit/runtime. Bundling was necessary in the very earliest days, when we were using .ts files, and in theory it has a benefit today in the form of slightly quicker installations, but in practice it adds considerable complexity.

It means that in order to test the src code locally without rebuilding on every change, we need to have different code paths (switching on process.env.BUNDLED) that are rather cryptic. It reduces the quality of stack traces. It adds a build step that isn't really necessary.

In #5778, it makes things very complicated indeed. In order for the new error and redirect helpers to work, we need to be able to detect, from a catch clause, that they were used. The best (and most TypeScript-friendly) way to do this is an instanceof check that uses the internal HttpError and Redirect classes used by those helpers. In the interests of not over-committing API surface area, these live in a private module. But this means that error and redirect imported from the public @sveltejs/kit module are referring to different copies of those classes than the code that does the instanceof checks, and there's really no way to fix that without introducing a bunch more complexity.

Unless, that is, we just get rid of the bundling altogether. Since SvelteKit has few dependencies, most of which have no or few dependencies of their own, I think the slight hit to installation time is more than justified by the substantial reduction in complexity.

@changeset-bot
Copy link

changeset-bot bot commented Aug 8, 2022

⚠️ No Changeset found

Latest commit: 12f65d1

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@Rich-Harris
Copy link
Member Author

(An aside: the ability to do this without losing type safety is one of the main reasons I'm a vocal proponent of using TypeScript via JSDoc, rather than via .ts files.)

"set-cookie-parser": "^2.4.8",
"sirv": "^2.0.2",
"tiny-glob": "^0.2.9",
"typescript": "^4.7.4",
Copy link
Member

Choose a reason for hiding this comment

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

oof

Copy link
Member

Choose a reason for hiding this comment

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

sorry, should have been more wordy 🙈
is it possible to not depend on typescript here? That's not small and even if users mostly are going to have it around during dev anyways, outright depending on it would lead to npm i --production pulling it too, which i think is not good / too heavy

Copy link
Member Author

Choose a reason for hiding this comment

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

this is the plan, yeah — it's only necessary for generated types, and if you don't have typescript in your app then there's no point in generating those types. so ideally we'll depend on the app's copy of typescript. i was just too lazy to do that

Rich-Harris added a commit that referenced this pull request Aug 15, 2022
* skeleton

* add moved info to files to be able to rewrite imports later

* add magic-string, start load extraction and manipulation logic

* move migration logic into new svelte-migrate package

* remove commented code

* lockfile

* move components, extract load

* +page.server.js and +server.js

* remove unused code

* uncomment prompt

* add some errors

* fix detection

* consistent casing

* not sure if typescript will ever fail to parse, but in any case its handled now

* handle error with module context

* add some comments

* update demo app

* update skeleton app

* migrate kit.svelte.dev

* fix dynamic import on windows, fix js/ts file ending, handle index.json.js endpoint case

* extract imports into Svelte migration task for QOL, don't adjust imports in standalone index edge case

* migrate amp app

* show message either way, could be other things gone wrong

* windooows

* more sophisticated auto migrations
- unpack props/body
- redirect/error in load
- standalone endpoint

* migrate test/apps/basics

* fix regex, dont show props message on error page, more sophisticated error/redirect checks, set-cookie suggestion

* pretend to make this more readable

* adjust import location for error and redirect

* update tests

* only show short automigration message on success, only migrate where we are confident it works, annotate non-migrated return statements, handle export { X } case

* shorten redirect/error if no second argument given

* use git mv if available

* update options tests

* update options-2 tests

* update writes tests

* migrate (most) prerendering/basics tests

* migrate more tests

* update adapter-static tests

* it begins (#5782)

* it begins

* update some more test fixtures

* update more tests

* more

* more

* more

* more

* last one

* need to add dot: true to catch routes inside .well-known

* fixes, update tests

* fix more tests

* fix various things

* all create_manifest_data unit tests passing

* update write_manifest

* fix

* remove config.kit.routes

* update RequestHandler type

* simplify endpoint code

* simplify some more stuff

* move respond code into render_page, where it belongs

* purge all references to stuff

* simplify

* vite build completes, albeit with a broken build

* vite dev now starts

* now rendering a blank page

* rendering an error page

* it renders html!

* tidy up

* get error page rendering correctly

* add placeholder setHeaders function

* replace CSRComponent with CSRPageNode

* fix build

* amp tests passing! now for the client

* fix create_manifest_data unit tests

* add error/redirect helpers

* rename @sveltejs/kit/data -> @sveltejs/kit

* add error/redirect to @sveltejs/kit types

* fixes

* fix a slew of typescript errors

* data loading belongs in load_node

* holy shit the tests are starting to pass

* update some tests

* start working on write_types

* science experiment

* typo

* i think it basically works

* expose ServerData as Data when no load is specified

* replace magic number

* handle missing GET separately from failure

* oops i think i did a bad git

* remove typings tests, these are no longer useful

* tidy

* use server data if no load is provided

* temporarily disable non-SSR tests

* apply headers

* update test

* allow load to return void

* remove some tests that no longer make sense

* remove some more unused tests

* this is fun. delete delete delete

* fix test

* handle accept:json requests

* fix test

* small tidy up

* tidy

* simplify

* simplify, improve efficiency

* simplify

* rename to disambiguate with client

* tweaks

* tidy up

* load data in parallel

* throw error pointing to migration if router folder exists but no routes are found

* load nodes on client in parallel

* handle redirect

* fix parent usage bug, avoid waterfall when awaiting parents, remove try-catch which would make subsequent catch never getting called

* error handling

* construct redirect URL properly

* fix redirects

* handle POST requests etc

* oof this took an embarrassingly long time to find

* add types

* update/remove some tests as appropriate

* implement $page.data on the server

* fix test

* handle invalid redirect statuses

* update test, fix off-by-one error

* fix a bunch more tests

* tweak tests

* include dynamically imported styles from +page.js in dev

* same but for build

* track cookies

* apply tracked cookies

* down to three failing SSR tests

* render errors

* all SSR tests in test/apps/basics passing

* fix write tests

* all SSR tests passing

* remove a couple of unused deps

* tidy up

* dont use instanceof checks

* fix off-by-one error in client

* client-side fetching, start of redirect/error handling

* re-add missing dist file

* for now, duplicate route.path logic where needed. in future, rethink manifest for this purpose

* fix parent data retrieval, add $page.data to client

* fix most prerendering tests

* add cache http-equiv

* test

* get prerender tests passing

Co-authored-by: Simon Holthausen <simon.holthausen@vercel.com>

* Docs for the new API (#5810)

* Update routing
- add overview of special files at the beginning (TODO: make this its own page?)
- update special file names
- shuffle endpoints and standalone endpoints to make it more clear what separates them (TODO: move each into its own page?)

* update layouts/error

* remove obsolete option

* updates to file names in various places

* update loading docs

* update sapper migration

* missed some

* clarify depends API

* Update documentation/docs/03-routing.md

Co-authored-by: Rich Harris <richard.a.harris@gmail.com>

* enable all tests

* Unbundling (#5853)

* tweak bundling strategy

* unbundle

* remove some unused deps

* simplify

* simplify further

* fix export map

* fix adapter-static tests

* format

* replace some TODOs

* only add param tag if param exists

* fix some type stuff

* fix some type stuff

* more type fixes

* reuse load_data

* fix imports. damn you vscode, you dont have to be like this

* fix some stuff

* remove unused import

* fix prefetchRoutes

* fix a handful more tests

* comment out likely obsolete setHeaders in test

* fix write_client_manifest

* fix off-by-one error for client error page rendering

* update page store when only data changed

* handle page endpoint without get

* reenable tests

* fix windows/vite path issue

* pass serialized server data to client

* fix uses tracking for page endpoints in client

* unfix the buggy drive letter fix

* set headers/etag for page endpoints

* add HttpError identifier to serialized version and make connection visible through types

* call handle_error in json request

* fix test

* omit json-data when ssr but no hydration

* remove obsolete caching logic from client, adjust test

* add migration link

* make typescript non-required again by lazy-loading it, fix package.files

* only create proxy if something was modified

* update pnpm lock

* always bundle @sveltejs/kit

* fix some write_types stuff

* this took waaaay too long to figure out

* prettier

* fix a bunch of typescript stuff

* shrug

* probably no point generating types if no ts?

* refactor write_types to support addition of layout types

* rename Data to PageData and LayoutData

* fix manifest generation with named layouts

* update tests

* generate LayoutLoad and LayoutData types

* windows fix, deduplicate, use AwaitedProperties

* write Errors type

* fix AwaitedProperties type, add AwaitedErrors type

* fix test

* fix test

* trying an env fix, not sure if that works on linux

* mhm

* temporary run only env test on CI

* fix type reference in create-svelte

* getting desperate

* even more scoped test execution

* check why posixify doesnt work

* Data -> PageData

* wtf, does this work now or not?

* remove unnecessary  migration warnings, add error for props -> data

* omg works, reenable all tests

* some docs progress

* fix file ending issue in write_types

* fix path resolution in write_types

* fix type error in generated d.ts file

* updates

* ooh it works

* sprinkle some types

* add checkJs

* get some pages building

* .ts -> .js

* simplify write_types

* more docs types stuff

* rename node.module to node.shared, which is more descriptive

* rename route.page to route.leaf

* return all route data with __data.json, not just the leaf

* load all server data in one go on the client. this should probably have a bunch of tests

* update types on file edit

* only write types that have changed

* debounce updates to manifest

* add parent data type inference. introduce a breaking change to the Load typing (another generic which is placed before an existing one)

* rename/tidy up some stuff

* fix

* fix some stuff

* set cookies on __data.json requests

* fix some stuff

* only load data from server if we have some

* remove obsolete todo

* update test

* lint

* fix prerendered __data.json

* add json helper

* use json helper in migration

* fixes

* center routing documentation on + files

* get docs building again

* update example

* tidy up

* update load docs

* document promise unwrapping

* typo

* remove obsolete logic which is now handled elsewhere

* implement invalidate()

* invalidate() should also loads without dependencies

* Update documentation/docs/05-load.md

Co-authored-by: Simon H <5968653+dummdidumm@users.noreply.github.com>

* Update documentation/docs/03-routing.md

* inject $types.d.ts files into code snippets where possible

* add a note about shared state. not 100% sure this is the best place for it but will do for now

* Update documentation/docs/05-load.md

* document set-cookie exception

* tweaks

* tweaks

* more tweaks

* cheat

* GET->load

* more tweaks

* fix create-svelte

* more tweaks

* Update documentation/docs/05-load.md

Co-authored-by: Simon H <5968653+dummdidumm@users.noreply.github.com>

* fixes

* remove outdated Get reference

* actions

* rewrite type names

* rename Load to PageLoad or LayoutLoad etc

* include filename in migration scripts

* updates

* fix search

* mention export let errors

* changesets

* fix types in create-svelte template

Co-authored-by: Simon Holthausen <simon.holthausen@vercel.com>
Co-authored-by: Simon H <5968653+dummdidumm@users.noreply.github.com>
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