👩‍💻 improved HydrogenSession typing.

[michenly]

WHY are these changes introduced?

The current typing for HydrogenSession is de-coupled from the implementation of HydrogenSession in project. This means the shape of the class could be very different from what @shopify/hydrogen expect session to look like.

WHAT is this pull request doing?

Export the typing for HydrogenSession from @shopify/hydrogen and use it to implement HydrogenSession in all the example projects.

The type is implemented using base session type from @remix-run/server-runtime which should ensure all session type from remix are supported. (Addressing concern from #1430 (comment))

Having HydrogenSession implemented from a base type will ensure the interface of this class is what we expect it to be when we use property from this class in hydrogen utility.

HOW to test your changes?

Any CookieSession class that implements HydrogenSession type, changed the interface so it no longer conform to the interface and see type error in editor.

example 1: changed return type of commit method

example 2: remove get method

Post-merge steps

Checklist

• I've read the Contributing Guidelines
• I've considered possible cross-platform impacts (Mac, Linux, Windows)
• I've added a changeset if this PR contains user-facing or noteworthy changes
• I've added tests to cover my changes
• I've added or updated the documentation

[michenly]

Does anyone know if there are other documentation I need to update with this change?

[frandiox]

I'd suggest renaming the project session to CookieSession and keep HydrogenSession type as is:

import {
 // ...
 type HydrogenSession,
} from '@shopify/hydrogen';
// ...

        CookieSession.init(request, [env.SESSION_SECRET]),

// ...
export class CookieSession implements HydrogenSession {
// ...

It might require more changes but it feels like a better update 🤔

[blittle]

Somewhat nit-picky, but I think that DefaultHydrogenSession doesn't really make sense as an identifier name. It is the interface that the developer must conform to. So it isn't really a "default". It is what they need to implement to ensure compatibility. IMO export class Session implements HydrogenSession makes more sense. But that means a bit more to modify.

[blittle]

Ahh, I see Fran above had a similar comment. Whether it is Session or CookieSession, I just don't like calling the type DefaultHydrogenSession in our examples.

[michenly]

OK I will update the name of the Session in our template. Was avoiding making a bigger change but if everyone are on board I will make it happen :)

[blittle]

This is a great change, thank you for pushing it!