Switch naive database implementation to PostgreSQL #4

ProchaLu · 2024-07-02T11:12:36Z

Closes #10

For the Expo React Native Guest List Example, we used a global array to save the guests. This PR updates the example and now uses PostgreSQL with Postgres.js and Ley.

TODO

Add @upleveled/ley, dotenv-safe, postgres and tsx to package.json
Add util/config.js and database/connect.ts file
Add migration file for guests
Fix db queries
Fix API routes
Try using postgres() directly instead of using the singleton pattern to test whether Expo API Routes suffer from the same problem with max PostgreSQL connections that Next.js does (check hot reloading) Dev mode hot reload loses database connection vercel/next.js#26427 (comment) - resolution: Expo API Routes also suffer from the same problem
- If Expo API Routes hot reloading behavior also has this problem we need the singleton pattern, switch globalThis var to local let var in IIFE closure and test development server Switch globalThis var to local let var in IIFE closure next-js-example-spring-2024-atvie#8 - resolution: IIFE gets re-initialized with hot reloading, we cannot use this

Co-authored-by: Karl Horky <karl.horky@gmail.com>

karlhorky · 2024-07-02T14:09:45Z

app/[id]+api.ts

as @ProchaLu mentioned, let's do another PR (stacked on top of this one) to move these to app/api/ (let's keep the files in place for this PR, to keep the diff simple)

deployment

where are we suggesting students deploy Expo API Routes + PostgreSQL to?

let's add a section to the React Native / Expo lecture notes about deployment (let's track that PR / issue also here)

let's do a video of this and integrate it with the current material

created a PR for the folder structure change to app/api Restructure to use /api folder with guestId #6

i. We suggest to deploy the Expo API Routes with PostgreSQL on Vercel.
ii. Created a PR for the Deployment section on the Learning Platform and will create a PR for the Lecture notes

https://github.com/upleveled/courses/pull/2571

https://github.com/upleveled/courses/pull/2546

karlhorky · 2024-07-02T14:14:47Z

database/connect.ts

as @ProchaLu mentioned, let's add a section to the React Native / Expo lecture notes (let's track that PR / issue also here)

let's do a video of this and integrate it with the current material

Created a PR for the React Native PostgreSQL lecture notes https://github.com/upleveled/courses/pull/2546

Created a small video

Eprince-hub · 2024-07-02T14:17:48Z

database/connect.ts

+function connectOneTimeToDatabase() {
+  if (!('postgresSqlClient' in globalThis)) {
+    globalThis.postgresSqlClient = postgres({
+      transform: {
+        ...postgres.camel,
+        undefined: null,
+      },
+    });
+  }
+
+  // Use Next.js Dynamic Rendering in all database queries:
+  //
+  // Wrap sql`` tagged template function to call `noStore()` from
+  // next/cache before each database query. `noStore()` is a
+  // Next.js Dynamic Function, which causes the page to use
+  // Dynamic Rendering
+  //
+  // https://nextjs.org/docs/app/building-your-application/rendering/static-and-dynamic-rendering
+  return ((
+    ...sqlParameters: Parameters<typeof globalThis.postgresSqlClient>
+  ) => {
+    return globalThis.postgresSqlClient(...sqlParameters);
+  }) as typeof globalThis.postgresSqlClient;
+}


As we don't need this noStore in expo, we may instead connect to the database like in the GraphQL example which is simpler

https://github.com/upleveled/graphql-example-spring-2024-atvie/blob/65f8a40fe857796e7cd649eb3ca5e4c8f2db3556/database/connect.ts#L16-L24

function connectOneTimeToDatabase() { if (!globalThis.postgresSqlClient) { globalThis.postgresSqlClient = postgres(postgresConfig); } return globalThis.postgresSqlClient; } // Connect to PostgreSQL export const sql = connectOneTimeToDatabase();

it seems like the current GraphQL example is missing a few updates from the Next.js example:

database/connect.ts

GraphQL example: https://github.com/upleveled/graphql-example-spring-2024-atvie/blob/65f8a40fe857796e7cd649eb3ca5e4c8f2db3556/database/connect.ts

Next.js example: https://github.com/upleveled/next-js-example-spring-2024-atvie/blob/main/database/connect.ts

util/config.js

GraphQL example: https://github.com/upleveled/graphql-example-spring-2024-atvie/blob/65f8a40fe857796e7cd649eb3ca5e4c8f2db3556/util/config.js

Next.js example: https://github.com/upleveled/next-js-example-spring-2024-atvie/blob/main/util/config.js

Updated in this PR
upleveled/graphql-example-spring-2024-atvie#1

…ng-2024-atvie into add-postgres

Co-authored-by: Karl Horky <karl.horky@gmail.com>

karlhorky · 2024-07-04T11:11:09Z

app/guests+api.ts

-export function GET(request: Request): Response {
+export async function GET(request: Request): Promise<Response> {


there's no way to type the response data of the Expo API response is there?

eg. like NextResponse in Next.js?

Accept generic type parameters for NextResponse vercel/next.js#45943

Add optional generic parameter to NextResponse vercel/next.js#47526

if not, we should probably:

see if it's possible to copy/paste a minimal implementation (eg. this TypeScript Playground version) to the Expo example repo (add to the Expo setup steps) - name could be something like ExpoApiResponse

open an Expo issue, if one doesn't already exist

I also created an issue to enforce these type checks in the ESLint config (once we have them):

Add enforcement of type checking in Expo API Routes eslint-config-upleveled#395

Generic type parameters for Expo API Routes Issue

Accept generic type parameters for Expo API Routes expo/expo#30521

Added the types for the ExpoApiResponse in this commit
98e83ec

In the previous commit, I added types, that I already had in my migrations file. Removed again
23be733

eslint-config-upleveled updated:

Enforce Expo API Route response type checking eslint-config-upleveled#447

https://github.com/upleveled/eslint-config-upleveled/releases/tag/v8.10.0

karlhorky · 2024-07-04T11:38:12Z

database/guests.ts

+import { Guest } from '../migrations/00000-createTableGuests';
+import { sql } from './connect';
+
+export const getGuests = async () => {


const ... = async () => arrow function style used here to be closer to the cache() database query function code we have in the Next.js example:

https://github.com/upleveled/next-js-example-spring-2024-atvie/blob/869d615738ee991fd80f31b44dfa3c4f79a7ce3d/database/animals.ts#L50-L62

Co-authored-by: Karl Horky <karl.horky@gmail.com>

karlhorky · 2024-10-24T20:33:22Z

app/(tabs)/index.tsx

+
+  useFocusEffect(
+    useCallback(() => {
+      const getGuests = async () => {


async function getGuests instead of arrow function

(for all functions inside useCallback following this pattern)

changed to functions
70e748d

karlhorky · 2024-10-24T20:35:54Z

app/(tabs)/index.tsx

+        try {
+          const response = await fetch('/api/guests', {
+            headers: {
+              Cookie: 'name=value',
+            },
+          });
+          const body: { guests: Guest[] } = await response.json();
+
+          setGuests(body.guests);
+        } catch (error) {
+          console.error('Error fetching guests', error);
+        }
+      };
+
+      getGuests().catch(() => {});


no need for the try/catch - you have the .catch() down below

inside of the .catch() is where the console.error should be

(for all functions inside useCallback following this pattern)

catch the errors now
70e748d

karlhorky · 2024-10-24T20:37:43Z

app/(tabs)/newGuest.tsx

+            try {
+              const body: { error: string } = await response.json();
+              if (body.error) {
+                errorMessage = body.error;
+              }
+            } catch {}


another try/catch that is swallowing errors - we should avoid this pattern

we want it to throw if there are errors

(check for all try/catch patterns to see if we need them)

Removed some try/catch and catch errors
70e748d

karlhorky · 2024-10-24T20:44:03Z

app/guests/[guestId].tsx

+  const [edit, setEdit] = useState<boolean>(false);
+  const [firstName, setFirstName] = useState<string>('');
+  const [lastName, setLastName] = useState<string>('');
+  const [attending, setAttending] = useState<boolean>(false);


these all don't need generic args

Suggested change

const [edit, setEdit] = useState<boolean>(false);

const [firstName, setFirstName] = useState<string>('');

const [lastName, setLastName] = useState<string>('');

const [attending, setAttending] = useState<boolean>(false);

const [edit, setEdit] = useState(false);

const [firstName, setFirstName] = useState('');

const [lastName, setLastName] = useState('');

const [attending, setAttending] = useState(false);

added in this commit
02412d3

karlhorky · 2024-10-24T20:46:10Z

database/guests.ts

+export const deleteGuestInsecure = async (id: number) => {
+  const [guest] = await sql<Guest[]>`
+    DELETE FROM guests
+    WHERE
+      id = ${id}


Suggested change

export const deleteGuestInsecure = async (id: number) => {

const [guest] = await sql<Guest[]>`

DELETE FROM guests

WHERE

id = ${id}

export const deleteGuestInsecure = async (guestId: Guest['id']) => {

const [guest] = await sql<Guest[]>`

DELETE FROM guests

WHERE

id = ${guestId}

added in this commit
02412d3

karlhorky · 2024-10-24T20:52:24Z

app/guests/[guestId].tsx

+export default function GuestPage() {
+  const { guestId } = useLocalSearchParams();
+
+  const [edit, setEdit] = useState<boolean>(false);


since it's a boolean, let's switch to the is prefix convention:

isEditing, setIsEditing

Change variable 02412d3

karlhorky

LGTM, thanks!

ProchaLu and others added 5 commits July 2, 2024 13:05

Switch to PostgreSQL

4df6497

Add linebreak

8c4922a

Revert createGuest to addGuest

e63fa70

Update Readme

bbc274e

Update migrations/00000-createTableGuests.ts

444e81f

Co-authored-by: Karl Horky <karl.horky@gmail.com>

karlhorky reviewed Jul 2, 2024

View reviewed changes

Eprince-hub reviewed Jul 2, 2024

View reviewed changes

ProchaLu and others added 8 commits July 2, 2024 16:25

Remove globalThis

826cb1b

Add libpg, safeQL, GitHub workflow

8fe7524

Add .env to GH workflow

7a3f5d4

Update database/connect.ts

9914548

Update connect.ts

0849efd

Merge branch 'main' of https://github.com/upleveled/expo-example-spri…

a5e3132

…ng-2024-atvie into add-postgres

Fix missing steps in CI

86d565a

Upgrade pnpm action

0d1e953

ProchaLu mentioned this pull request Jul 2, 2024

Restructure to use /api folder with guestId #6

Merged

ProchaLu added 3 commits July 3, 2024 14:17

Remove type user

4cb67ea

add type Guest to migration file

832a0bb

Update imports for type Guest

5ef0c74

upleveled deleted a comment from ProchaLu Jul 3, 2024

ProchaLu and others added 3 commits July 4, 2024 12:46

Update .github/workflows/ci.yml

ae543cf

Co-authored-by: Karl Horky <karl.horky@gmail.com>

Update .github/workflows/ci.yml

0295119

Co-authored-by: Karl Horky <karl.horky@gmail.com>

Update .github/workflows/ci.yml

9f8bbca

Co-authored-by: Karl Horky <karl.horky@gmail.com>

karlhorky reviewed Jul 4, 2024

View reviewed changes

karlhorky mentioned this pull request Jul 4, 2024

Add enforcement of type checking in Expo API Routes upleveled/eslint-config-upleveled#395

Closed

karlhorky reviewed Jul 4, 2024

View reviewed changes

ProchaLu added 2 commits July 4, 2024 13:49

Add missing dependencies for prettier

fb94173

Add custom property to avoid ts error

7101919

karlhorky mentioned this pull request Jul 4, 2024

Allow empty interfaces with single extends upleveled/eslint-config-upleveled#397

Merged