Disable persistence by default (unless --enable-persistence passed) #321
Conversation
🦋 Changeset detectedLatest commit: f4a3c9c The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
geelen
force-pushed
the
glen/disable-persistence-by-default
branch
from
b32b364
to
4bcb4a3
Compare
|
This resolves #320 |
|
call it |
| { | ||
| "printWidth": 80, | ||
| "singleQuote": false, | ||
| "semi": true | ||
| } |
There was a problem hiding this comment.
Perhaps this change is something to discuss in a separate PR?
There was a problem hiding this comment.
Oh I see, this is to match the current settings, and override any user-level config that a developer might have...?
There was a problem hiding this comment.
Yeah my ~/.prettierrc is set to the opposite of all of these, so running prettier on anything exploded the diffs. I ran npm run prettify after adding this and it changed nothing, except the npm run check:format on CI still failed. Not sure why...
packages/wrangler/src/dev.tsx
Outdated
| // break; | ||
| // } | ||
| // } | ||
| // ); |
There was a problem hiding this comment.
Is this supposed to be commented out?
There was a problem hiding this comment.
Damn sorry that was me trying to fix something else... Will redo and repush
packages/wrangler/src/dev.tsx
Outdated
| @@ -40,6 +40,7 @@ export type DevProps = { | |||
| initialMode: "local" | "remote"; | |||
| jsxFactory: undefined | string; | |||
| jsxFragment: undefined | string; | |||
| enablePersistence: boolean; | |||
There was a problem hiding this comment.
BIKESHED: enableLocalPersistence or localModePersistence or something that signifies that this only affects local mode?
geelen
force-pushed
the
glen/disable-persistence-by-default
branch
from
fbc9a0a
to
a499762
Compare
There was a problem hiding this comment.
LGTM - the failure is a TypeScript one...
Error: [check:type ] packages/wrangler/src/__tests__/dev.test.tsx(49,6): error TS2741: Property 'enableLocalPersistence' is missing in type '{ name: string | undefined; entry: string; port: number | undefined; buildCommand: { command?: string | undefined; cwd?: string | undefined; watch_dir?: string | undefined; }; format: CfScriptFormat; ... 9 more ...; bindings: { ...; }; }' but required in type 'DevProps'.
Looks like you need to update dev.test.ts
geelen
force-pushed
the
glen/disable-persistence-by-default
branch
from
f38b835
to
5a47d35
Compare
.changeset/two-coins-compare.md
Outdated
| "wrangler": patch | ||
| --- | ||
|
|
||
| Disabled local persistence by default & added --experimental-enable-local-persistence flag |
There was a problem hiding this comment.
Oh last thing... Could you add a breaking changes notification here... Something like:
BREAKING CHANGE:
When running `dev` locally any data stored in KV, Durable Objects or the cache are no longer persisted between sessions by default.
To turn this back on add the `--experimental-enable-local-persistence` at the command line.
5a47d35
to
25d1d1f
Compare
feat: Added —experimental-enable-local-persistence flag to turn back on chore: added project-level .prettierrc, otherwise system-wide settings cause massive diffs chore: added changeset
geelen
force-pushed
the
glen/disable-persistence-by-default
branch
from
25d1d1f
to
f4a3c9c
Compare
|
@geelen - oops: |
This fix passes the correct worker name to `syncAssets` during `wrangler dev`. This function uses the name to create the backing kv store for a Workers Sites definition, so it's important we get the name right. I also fixed the lint warning introduced in #321, to pass `props.enableLocalPersistence` as a dependency in the `useEffect` call that starts the "local" mode dev server.
|
fix for the lint warning included in #333 |
This fix passes the correct worker name to `syncAssets` during `wrangler dev`. This function uses the name to create the backing kv store for a Workers Sites definition, so it's important we get the name right. I also fixed the lint warning introduced in #321, to pass `props.enableLocalPersistence` as a dependency in the `useEffect` call that starts the "local" mode dev server.
Thought it was time to pull my finger out and send a PR, now that I don't have the "but I don't know Rust" defence to lean back on...