Skip to content

Convert task list apps to TypeScript - #1

Open
dianadenwik wants to merge 1 commit into
HackYourAssignment:mainfrom
dianadenwik:typescript
Open

dianadenwik wants to merge 1 commit into
HackYourAssignment:mainfrom
dianadenwik:typescript

Conversation

@dianadenwik

Copy link
Copy Markdown

No description provided.

Comment thread task-list-ts/src/main.ts
id: nextId++,
title: title,
completed: false,
createdAt: new Date()

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice that you added the Date and also showing with toLocaleDateString()!

Comment thread task-list-ts/src/main.ts
list.appendChild(li);
});
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All function very typed, well done!

Comment thread task-list-ts/src/main.ts
Comment on lines +16 to +19
const form = document.getElementById('task-form')as HTMLFormElement;;
const input = document.getElementById('task-input')as HTMLInputElement;
const list = document.getElementById('task-list')as HTMLUListElement;
const emptyMessage = document.getElementById('empty-message')as HTMLParagraphElement;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice in using the HTML types. I would only remove the double semi-colon in line 16 and and a space before as

Suggested change
const form = document.getElementById('task-form')as HTMLFormElement;;
const input = document.getElementById('task-input')as HTMLInputElement;
const list = document.getElementById('task-list')as HTMLUListElement;
const emptyMessage = document.getElementById('empty-message')as HTMLParagraphElement;
const form = document.getElementById('task-form') as HTMLFormElement;
const input = document.getElementById('task-input') as HTMLInputElement;
const list = document.getElementById('task-list') as HTMLUListElement;
const emptyMessage = document.getElementById('empty-message') as HTMLParagraphElement;

Comment thread task-list-ts/src/main.ts
id: number
title: string
completed: boolean
createdAt: Date

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add a tab here to follow the formatting. I see also that in some places of your files you use semicolon, and others not. Define a pattern is always good for readbility

Comment thread task-list-ts/src/main.ts
});

const date = document.createElement('small');
date.textContent = task.createdAt.toLocaleDateString()

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Additional TAB here

@@ -0,0 +1,23 @@
{
"compilerOptions": {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A tip for TS configuration: when addinf the propery strict: true and will get all the null returns and this secures that all functions has a type linked

@rafasilpereira rafasilpereira left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The build generates asset paths as /assets/index.js (absolute), which breaks when dist/index.html is opened and that doesn't root at the project folder. Adding a vite.config.ts with base: './' fixes this so the built app works anywhere:

import { defineConfig } from 'vite'
export default defineConfig({
  base: './',
})

@rafasilpereira rafasilpereira added the Reviewed This assignment has been reivewed by a mentor and a feedback has been provided label May 25, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Reviewed This assignment has been reivewed by a mentor and a feedback has been provided

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants