Migrate to Next.js 14 app directory #156

ProchaLu · 2024-01-02T08:34:00Z

Closes #152

This PR migrates the portfolio to Next.js 14 and the app directory structure. It imports the necessary changes from the original portfolio and fixes any errors encountered during the process.

TODO

Pull Next Starter Peacock PR Next 14, UI improvements vickonrails/next-starter-peacock#21
Add UpLeveled ESLint
Fix all Preflight errors
Fix all Typescript errors
Add data-test-id for Playwright test

Compare the current changes with the upstream repo:

https://github.com/vickonrails/next-starter-peacock/compare/vickonrails:next-starter-peacock:master...upleveled:next-starter-peacock:update-to-next-14?diff=unified&w=

Bumps [ini](https://github.com/isaacs/ini) from 1.3.5 to 1.3.8. - [Release notes](https://github.com/isaacs/ini/releases) - [Commits](npm/ini@v1.3.5...v1.3.8) Signed-off-by: dependabot[bot] <[email protected]>

Bumps [y18n](https://github.com/yargs/y18n) from 4.0.0 to 4.0.3. - [Release notes](https://github.com/yargs/y18n/releases) - [Changelog](https://github.com/yargs/y18n/blob/y18n-v4.0.3/CHANGELOG.md) - [Commits](yargs/y18n@v4.0.0...y18n-v4.0.3) Signed-off-by: dependabot[bot] <[email protected]>

Bumps [ssri](https://github.com/npm/ssri) from 6.0.1 to 6.0.2. - [Release notes](https://github.com/npm/ssri/releases) - [Changelog](https://github.com/npm/ssri/blob/v6.0.2/CHANGELOG.md) - [Commits](npm/ssri@v6.0.1...v6.0.2) Signed-off-by: dependabot[bot] <[email protected]>

Bumps [elliptic](https://github.com/indutny/elliptic) from 6.5.3 to 6.5.4. - [Release notes](https://github.com/indutny/elliptic/releases) - [Commits](indutny/elliptic@v6.5.3...v6.5.4) Signed-off-by: dependabot[bot] <[email protected]>

chore(deps): bump ini from 1.3.5 to 1.3.8

chore(deps): bump y18n from 4.0.0 to 4.0.3

chore(deps): bump ssri from 6.0.1 to 6.0.2

…-6.5.4 chore(deps): bump elliptic from 6.5.3 to 6.5.4

Configure Renovate

fix(deps): pin dependencies

Update dependencies

…peacock

Fix typo

ProchaLu · 2024-01-03T08:48:10Z

components/chips.tsx

+    <ul className="list-none flex flex-wrap pl-0">
+      {items.map((tag) => (
+        <li
+          key={`items-${tag}`}


Used template literals including prefixes for the values of key props

ProchaLu · 2024-01-03T08:50:55Z

components/experiment.tsx

-`;
-
-const ExperimentItem = ({ experiment }: Props) => {
+export function ExperimentItem({ experiment }: { experiment: Experiment }) {


Added new created type from line 9 to avoid any

created a PR in upstream

Update TypeScript types vickonrails/next-starter-peacock#25

ProchaLu · 2024-01-03T08:53:34Z

components/link.tsx

+import { AllHTMLAttributes } from 'react';
+import { cn } from '../utils/cn';


Removed imported HTMLAttributes since it is never used, and changed to relative imports

ProchaLu · 2024-01-03T08:54:44Z

components/nav/mobile-nav.tsx

+                <a
+                  href="https://github.com/vickOnRails/next-starter-peacock"
+                  target="_blank"
+                  rel="noopener noreferrer"


Changed typo from norefferrer to noreferrer

Any of these changes can also be a separate new PR to upstream

Created a small PR to upstream.

Fix noreferrer typo vickonrails/next-starter-peacock#23

ProchaLu · 2024-01-03T08:55:44Z

components/nav/nav.tsx

+  if (external) {
+    return (
+      <a
+        data-test-id={dataTestId}


Add data-test-id for Playwright testing

Removed all data-test-ids

24018f3

await page .locator('div:has-text("WorksArticlesNotesAbout") a') .nth(4) .getAttribute('href'),

doesn't it make more sense to use nav here instead of div? it seems there are 2 navs, be sure to write a good selector that will select the correct one

I guess using the .nth() is also a bit brittle

probably you can do a better version using just page.locator(...).getAttribute('href')

ProchaLu · 2024-01-03T08:56:27Z

components/notes/notes.tsx

      {notes.map((note) => (
        <Note
-          key={`note-${note.id}`}
+          id={note.id}
+          key={`note-${note.slug}`}


Used template literals including prefixes for the values of key props

ProchaLu · 2024-01-03T08:58:09Z

components/works/work.tsx

+import { IContent } from '../../utils/content';
+
+// TODO: use some type of hash function to generate the colors
+const colorsLookup = {


Changed COLORS_LOOKUP to camelCase colorsLookup

ProchaLu · 2024-01-03T09:01:14Z

components/works/work.tsx

+      href={`/works/${slug}`}
+      className={cn(
+        'group rounded-xl select-none no-underline flex flex-col lg:py-10 w-full md:flex-row transition-transform hover:cursor-pointer hover:scale-[102%] active:scale-95',
+        colorsLookup[slug as keyof typeof colorsLookup],


Added a type to avoid any. slug is cast to the type keyof typeof colorsLookup, telling TS that the slug is a key of the colorsLookup object.

ProchaLu · 2024-01-03T09:02:51Z

utils/content-types.ts

@@ -0,0 +1,31 @@
+import { IContentType } from './content';
+
+export const contentTypesMap: Map<IContentType, any> = new Map([


Changed variable CONTENT_TYPES_MAP to use camelCase

ProchaLu · 2024-01-03T09:03:53Z

utils/content.ts

 import fs from 'node:fs';
 import path from 'node:path';


updated the imports from import fs form 'fs' to import fs form 'node:fs'. The same for path

All of these node: changes you added can be added to a 3rd new separate PR to the upstream vickonrails repo - then our repo doesn't diverge as much

Created a PR for the upstream changes

Update Node.js module imports vickonrails/next-starter-peacock#24

ProchaLu · 2024-01-03T09:07:56Z

utils/content.ts

+  tags?: string[];
+  category?: string;


Add tags and category to type IContent since these values can be optional and this type is used on multiple occasions.

Should this be PRed to upstream vickonrails?

created a PR in upstream

Update TypeScript types vickonrails/next-starter-peacock#25

ProchaLu · 2024-01-03T09:12:59Z

utils/content.ts

+ * Sorts content by their dates
+ * @param a {Date} - Date of post 1
+ * @param b {Date} - Date of post 2
 */

 export const sortByDate = (a: { date?: Date }, b: { date?: Date }) => {


Moved the function up to avoid 'sortByDate' being used before it was defined.

ProchaLu · 2024-01-03T09:35:56Z

utils/content.ts

-    .map((contentFile) => {
-      const filePath = `${contentDir}/${contentFile}`;
-      const rawContent = fs.readFileSync(filePath, {
+    .map((contentItem) => {
+      const contentPath = `${contentDir}/${contentItem}`;
+      const rawContent = fs.readFileSync(contentPath, {


changed the variable names since they were already declared in line 2 and line 196

ProchaLu · 2024-01-03T09:39:04Z

utils/content.ts

        id: uuid(),
-      };
+      } as IContent;


Added type assertion because for TS the type returned had previewImage and id but this was wrong because in lines 207-209 we spread the data object and added previewImage and id

hmm, let's try to avoid a type assertion if possible (for all type assertions)

usually it means that the types need to be more strict somewhere

eg. maybe the data type:

https://github.com/upleveled/next-starter-peacock/pull/156/files/572c51dac06281f128f709a7ab809681c053718e#r1443651712

ProchaLu · 2024-01-03T09:40:12Z

utils/content.ts

@@ -221,15 +253,12 @@ export const getContentWithTag = (tag: string, contentType: IContentType) => {
        ...data,
        previewImage: data.previewImage || '/images/image-placeholder.png',
        id: uuid(),
-      };
+      } as IContent;


Added type assertion because for TS the type returned had previewImage and id but this was wrong because in lines 253-255 we spread the data object and added previewImage and id

hmm, let's try to avoid a type assertion if possible (for all type assertions)

usually it means that the types need to be more strict somewhere

eg. maybe the data type:

https://github.com/upleveled/next-starter-peacock/pull/156/files/572c51dac06281f128f709a7ab809681c053718e#r1443651712

ProchaLu · 2024-01-03T09:50:03Z

utils/content.ts

-      const rawContent = fs.readFileSync(filePath, {
+    .filter((content) => content.endsWith('.md'))
+    .map((content) => {
+      const contentPath = `${contentDir}/${content}`;


changed the variable names since it was already declared

ProchaLu · 2024-01-03T09:50:58Z

utils/content.ts

@@ -277,15 +306,16 @@ export const getContentInCategory = (
        ...data,
        previewImage: data.previewImage || '/images/image-placeholder.png',
        id: uuid(),
-      };
+      } as IContent;


Added type assertion because for TS the type returned had previewImage and id but this was wrong because in lines 306-308 we spread the data object and added previewImage and id

Interesting, wonder why we need this... What are the properties in the data object?

The initial type when hovering over data is

const data: { [key: string]: any; }

These are the properties that data can have, some properties are optional

title: string; slug: string; basePath: string; date: Date; draft?: boolean; selectedWork?: boolean; description?: string; previewImage?: string; tags?: string[]; category?: string;

why isn't data this type? maybe it should have this type applied to it instead?

ProchaLu · 2024-01-03T09:51:44Z

utils/rss.ts

+    });
+
+    articlesContent.sort(sortByDate).forEach((contentItem: IContent) => {
+      const { title, previewImage, date, slug, description } = contentItem;


removed id, because it was declared but never used

Anything unused - eg. a destructured property like this or an unused import - can be PRed to upstream in a separate new PR

created a PR in upstream

Remove unused variables and imports vickonrails/next-starter-peacock#26

karlhorky

Approving to unblock merging before we finish the feedback - I'll finish the remaining feedback afterwards and we can do follow-up PRs if necessary

dependabot bot and others added 30 commits April 29, 2021 09:05

chore(deps): bump ini from 1.3.5 to 1.3.8

d14cf7b

Bumps [ini](https://github.com/isaacs/ini) from 1.3.5 to 1.3.8. - [Release notes](https://github.com/isaacs/ini/releases) - [Commits](npm/ini@v1.3.5...v1.3.8) Signed-off-by: dependabot[bot] <[email protected]>

chore(deps): bump elliptic from 6.5.3 to 6.5.4

92546bd

Bumps [elliptic](https://github.com/indutny/elliptic) from 6.5.3 to 6.5.4. - [Release notes](https://github.com/indutny/elliptic/releases) - [Commits](indutny/elliptic@v6.5.3...v6.5.4) Signed-off-by: dependabot[bot] <[email protected]>

Add renovate.json

71f01e7

Merge pull request #1 from upleveled/dependabot/npm_and_yarn/ini-1.3.8

0474e25

chore(deps): bump ini from 1.3.5 to 1.3.8

Merge pull request #2 from upleveled/dependabot/npm_and_yarn/y18n-4.0.3

9f500dd

chore(deps): bump y18n from 4.0.0 to 4.0.3

Merge pull request #3 from upleveled/dependabot/npm_and_yarn/ssri-6.0.2

37b0488

chore(deps): bump ssri from 6.0.1 to 6.0.2

Merge pull request #4 from upleveled/dependabot/npm_and_yarn/elliptic…

aace76e

…-6.5.4 chore(deps): bump elliptic from 6.5.3 to 6.5.4

Merge pull request #5 from upleveled/renovate/configure

0d18111

Configure Renovate

fix(deps): pin dependencies

1b4d927

Merge pull request #6 from upleveled/renovate/pin-dependencies

f59562e

fix(deps): pin dependencies

chore(deps): update dependency @types/node to v14.14.43

2d84383

Merge pull request #7 from upleveled/renovate/node-14.x

c9d21bc

Add pinned dependencies

a187311

Merge branch 'master' into original-pinned-dependencies

9de0d40

Add pins to dependencies

3017835

Merge pull request #8 from upleveled/original-pinned-dependencies

9ab3254

Update dependencies

feat: add plausible analytics

88b9c00

fix: use url for post id instead of randomly generated id. fixes #10

a682fdf

chore: remove plausible analysis ��‍♂️

b862cd4

fix: use url for post id instead of randomly generated id. fixed #10

6ae99ad

Merge branch 'master' of https://github.com/vickOnRails/next-starter-…

58eef6e

…peacock

Fix typo

3b3443f

Change react-tilt to react-parallax-tilt

f7c84a4

Revert to double quotes

c5cfd0e

Update options

a6d723c

Merge pull request #12 from karlhorky/patch-1

7ef8680

Fix typo

chore: upgrade packages

4744123

refactor: remove <a> tags from <Link>

4c6ee1c

ProchaLu commented Jan 3, 2024

View reviewed changes

Add line break

572c51d

ProchaLu commented Jan 3, 2024

View reviewed changes

ProchaLu added 3 commits January 4, 2024 16:36

Remove empty line

3048cad

Remove empty line

98ad7ca

Refactor test, remove data-test-id

24018f3

karlhorky approved these changes Jan 4, 2024

View reviewed changes

ProchaLu added 2 commits January 5, 2024 09:06

Merge branch 'main' into update-to-next-14

d9b01c0

Update lockfile

78e3e26

ProchaLu merged commit a1af9f5 into main Jan 5, 2024
3 checks passed

ProchaLu deleted the update-to-next-14 branch January 5, 2024 08:18

upleveled deleted a comment from ProchaLu Jan 6, 2024

ProchaLu mentioned this pull request Jan 6, 2024

Restore Renovate Bot configuration file #162

Merged

		import { AllHTMLAttributes } from 'react';
		import { cn } from '../utils/cn';

		@@ -0,0 +1,31 @@
		import { IContentType } from './content';

		export const contentTypesMap: Map<IContentType, any> = new Map([

Migrate to Next.js 14 app directory #156

Migrate to Next.js 14 app directory #156

Conversation

ProchaLu commented Jan 2, 2024 • edited Loading

TODO

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

karlhorky Jan 3, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

karlhorky Jan 6, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

karlhorky Jan 6, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

karlhorky left a comment

Choose a reason for hiding this comment

ProchaLu commented Jan 2, 2024 •

edited

Loading

karlhorky Jan 3, 2024 •

edited

Loading

karlhorky Jan 6, 2024 •

edited

Loading

karlhorky Jan 6, 2024 •

edited

Loading