graphile / starter

Opinionated SaaS quick-start with pre-built user account and organization system for full-stack application development in React, Node.js, GraphQL and PostgreSQL. Powered by PostGraphile, TypeScript, Apollo Client, Graphile Worker, Graphile Migrate, GraphQL Code Generator, Ant Design and Next.js
https://graphile-starter.herokuapp.com
Other
1.75k stars 220 forks source link

Update express-session and add custom returnTo property #221

Closed jackbravo closed 3 years ago

jackbravo commented 3 years ago

Description

Also had some errores while updating express-session to latest release.

Specifically because of this change:

https://github.com/DefinitelyTyped/DefinitelyTyped/blob/master/types/express-session/index.d.ts#L202

from:

Reported here: https://github.com/DefinitelyTyped/DefinitelyTyped/issues/49484

Performance impact

Security impact

Checklist

benjie commented 3 years ago

It seems the updates to the lockfile have caused cascading failures in the cypress tests. Apparently someone's not been adhering to semver :/ Those issues will need fixing before this can be merged; but I think that's probably beyond the scope of this PR. I'm going to leave this open until I have time to address this breakage, but thank you for sharing the fix :+1:

lukeramsden commented 3 years ago

From the Cypress screenshots it seems to be a trailing slash issue.

https://github.com/vercel/next.js/pull/15768

Could be this? Or related changes in Next.js 9.x. Looks like they've moved to a new trailing slash API, and then redirected the old exportTrailingSlash to enable to new one?

lukeramsden commented 3 years ago

exportTrailingSlash: true -> trailingSlash: false in next.config.js seems to fix the issue on my starter-derived project, maybe one of you could make that change on this PR and see if that fixes the E2E tests?

jackbravo commented 3 years ago

It worked @lukeramsden ! 🎉

@benjie this seems ready now.

benjie commented 3 years ago

Awesome; merged into #222

benjie commented 3 years ago

Sorry for the delay; took a long time to track down a bug that turned out to be next build not working quite right in NODE_ENV=test