-
Notifications
You must be signed in to change notification settings - Fork 61
[Package] PowerSync Nuxt Module #797
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
base: main
Are you sure you want to change the base?
Conversation
… and initial setup
…Supabase Todo List demo
|
|
Not sure what the chanset should include when releasing a new package |
stevensJourney
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just sharing some initial comments for some items which stood out from an initial review.
| @click="navigateTo('/')" | ||
| > | ||
| <img | ||
| src="https://cdn.prod.website-files.com/67eea61902e19994e7054ea0/67f910109a12edc930f8ffb6_powersync-icon.svg" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this link copied from some site, or, where is it managed? Could we use an asset for this?
| 1. [Create a new project on the Supabase dashboard](https://supabase.com/dashboard/projects). | ||
| 2. Go to the Supabase SQL Editor for your new project and execute the SQL statements in [`db/seed.sql`](db/seed.sql) to create the database schema, PowerSync replication role, and publication needed for PowerSync. | ||
|
|
||
| **Important:** When connecting PowerSync to your Supabase database, you'll use the `powersync_role` credentials instead of the default Supabase connection string. This role has the necessary replication privileges and bypasses Row Level Security (RLS). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should probably add a note to update the password in db/seed.sql before running?
| └── nuxt.config.ts # Nuxt configuration | ||
| ``` | ||
|
|
||
| ## Learn More |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be nice if this demo had a local Supabase config and quickstart for local development - which can be used to start the demo in only a few commands - like the YJS Demo has here.
|
|
||
| worker: { | ||
| format: 'es', | ||
| plugins: () => [wasm(), topLevelAwait()], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe you should be able to remove the topLevelAwait plugin these days - due to the BSON lib no longer requiring it. It's only our list of todos for other demos.
| "devDependencies": { | ||
| "@nuxt/devtools": "^1.0.6", | ||
| "@nuxt/eslint": "^1.9.0", | ||
| "@supabase/supabase-js": "2.75.0", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: This should probably be a dependency?
| @@ -0,0 +1,27 @@ | |||
| import { column, Schema, Table } from '@powersync/web' | |||
|
|
|||
| export const local_bucket_data = new Table( | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: We typically name these kinds of constants in upper case.
| /** | ||
| * Record fields from downloaded data, then build a schema from it. | ||
| */ | ||
| export class DynamicSchemaManager { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks like a copy from tools/diagnostics-app? It would be nice if we had this functionality in some shared location.
| // broadcastLogs: true, // need to be enabled for multitab support | ||
| // } | ||
|
|
||
| // @ts-expect-error - type error because we are forcing the vfs to be the OPFSCoopSyncVFS |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems fishy. Why do we need to make this force assertion?
| // override settings to disable multitab as we can't use it right now | ||
| // options.flags = { | ||
| // ...options.flags, | ||
| // enableMultiTabs: true, // to support multitabe we need to write our won worker implementation |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you're overriding the PowerSyncDatabase, it might be possible (with some minor modifications) to actually implement your own web worker implementation, using the current primitives, which contains the required sync hooks.
| import type { DynamicSchemaManager } from './DynamicSchemaManager' | ||
| import type { Ref } from 'vue' | ||
| /** | ||
| * Tracks per-byte and per-operation progress for the Rust client. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just sharing this again. I'd really prefer we did not duplicate these implementations.
|
|
||
| You can use either PowerSync Cloud or self-host PowerSync: | ||
|
|
||
| - **PowerSync Cloud**: [Create a new project on the PowerSync dashboard](https://powersync.journeyapps.com/) and connect it to your Supabase database using the `powersync_role` credentials created in step 2. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: should probably link to https://dashboard.powersync.com/
| vite: { | ||
| plugins: [topLevelAwait()], | ||
| optimizeDeps: { | ||
| exclude: ['@journeyapps/wa-sqlite', '@powersync/web', '@powersync/common', '@powersync/vue', '@powersync/kysely-driver'], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need to exclude all @powersync/* packages from optimisation? AFAIK, the only ones that need to be excluded are @powersync/web and @journeyapps/wa-sqlite, since those packages include web workers and wasm respectively - the others should be fine to have optimised.
| class="p-0" | ||
| @click="sign = sign === 'up' ? 'in' : 'up'" | ||
| > | ||
| {{ sign === "in" ? "Sign up" : "Sign in" }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like this condition might be reversed.
| // title to display in the tab | ||
| title: 'Powersync Inspector', | ||
| // any icon from Iconify, or a URL to an image | ||
| icon: 'https://cdn.prod.website-files.com/67eea61902e19994e7054ea0/67f910109a12edc930f8ffb6_powersync-icon.svg', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should maybe be a file in 'assets' instead of a link - not too fussed, though
| import { computedAsync } from '@vueuse/core' | ||
| import { usePowerSyncInspector } from './usePowerSyncInspector' | ||
|
|
||
| type JSONValue |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The same JSON types are declared in src/module.ts. It might make sense to move these to a types.ts file or similar.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not entirely sure what the benefit of a dedicated script is over using npx npkill or even rm -rf demos/**/node_modules
This PR introduces a new
nuxtpackage to offer first-class support for Nuxt with PowerSync.The PR also introduces a new demo showcasing a reference implementation showing PowerSync + Nuxt 4 + Supabase