chore: update to fresh@1.2.0
#274
Conversation
|
makes sense. let me investigate these two new features and try to use them. |
|
Cool. Hopefully, adding these in makes things easier/better. If that's not the case, feel free to leave them out. |
|
@iuioiua , can you check if this last commit makes sense? For the feature that allows sharing signals between islands: not sure if this is useful here so far. We only have the VoteButton.tsx island and I couldn't see much advantage in having signals being passed via props there. What do you think? |
components/PageSelector.tsx
Outdated
| @@ -17,7 +17,7 @@ export default function PageSelector( | |||
| max={props.lastPage} | |||
| value={props.currentPage} | |||
| // @ts-ignore: this is valid HTML | |||
| onchange="this.form.submit()" | |||
| onchange={(e) => this.form.submit()} | |||
There was a problem hiding this comment.
| onchange={(e) => this.form.submit()} | |
| onchange={() => this.form.submit()} |
If the event parameter isn't used, it can be omitted.
routes/_app.tsx
Outdated
twind.config.ts
Outdated
| @@ -1,6 +1,6 @@ | |||
| // Copyright 2023 the Deno authors. All rights reserved. MIT license. | |||
| import { Options } from "$fresh/plugins/twindv1.ts"; | |||
| import { defineConfig, Preset } from "twind"; | |||
| import { defineConfig, Preset } from "https://esm.sh/@twind/core@1.1.3"; | |||
There was a problem hiding this comment.
Can you please revert? I don't think this is needed.
There was a problem hiding this comment.
Can you please confirm this here?
I needed to change this because once I upgraded fresh to 1.2, defineConfig and Preset were not defined in "twind" anymore.
This guide here (https://fresh.deno.dev/docs/examples/using-twind-v1) also imports defineConfig from core@1.1.3
If we remove the ctx.state, the sessionId doesn't get passed to the _app.tsx and the Layout component doesn't change as it is supposed to change (when the user is logged in, show 'account' in the navbar) |
There was a problem hiding this comment.
I've made a couple of tweaks, and now, LGTM! I'll land merge as-is now. I've asked whether creating the signal within the component or island is best. I'll make an issue if we should proceed with that or not.
Thanks for this, @brunocorrea23!
#260
For some reason when I updated fresh version to 1.2, the button on the island VoteButton.tsx stopped working. I had to remove the disabled={!IS_BROWSER}. Doesn't seem to have any impact.