Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[DRAFT] Update dependencies, add prettier/eslint for consistent style, add more type safety #127

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

epsilonhalbe
Copy link

I noticed the dependencies were a bit out of date and the style was not very consistent. As well as a few as ... and ts-ignore.

I moved the ts file into a src folder and extracted default values and the domain model.

I ran the linter & formatter on the codebase, I tried to make it do as little changes as possible but the mix of commas/semicolons/quotes needed some changes to be unified.

the formatting & linting is cnotained in a single commit so it can be reverted more easily and review of unrelated content is easier.

charset = utf-8
end_of_line = lf
insert_final_newline = true
indent_style = space
Copy link
Author

Choose a reason for hiding this comment

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

I got warnings of mixed tabs and spaces => changed this to spaces

@@ -1,2 +1,2 @@
npm node_modules
Copy link
Author

Choose a reason for hiding this comment

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

I think this was wrong, there is no build directory and npm node_modules doesn't make sense

import { Plugin } from 'obsidian'
import HeatmapCalendarSettingsTab from 'settings'

declare global {
Copy link
Author

Choose a reason for hiding this comment

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

adding the global declare removes the need for ts-ignore when accessing the window object


const intensities = calEntries
.map((e: Entry) => e.intensity)
.filter((intensity): intensity is number => intensity !== undefined)
Copy link
Author

Choose a reason for hiding this comment

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

this is a neat trick - to type filters for undefined. intensities with this has type number[] inferred by the compiler

parent: heatmapCalendarGraphDiv,
})

const months = ['Jan', 'Feb', 'Mar', 'Apr', 'May', 'Jun', 'Jul', 'Aug', 'Sep', 'Oct', 'Nov', 'Dec']
Copy link
Author

Choose a reason for hiding this comment

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

simplifies the previous duplication

}

async onload(): Promise<void> {
const settings = await this.loadSettings()
Copy link
Author

Choose a reason for hiding this comment

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

settings is only used here - no need for a global variable.

@@ -80,52 +47,54 @@ export default class HeatmapCalendar extends Plugin {
return this.clamp(mapped, outMin, outMax)
}

getWeekdayShort(dayNumber: number): string {
return new Date(1970, 0, dayNumber + this.settings.weekStartDay + 4).toLocaleDateString('en-US', {
getWeekdayShort(weekStartDay: number, dayNumber: number): string {
Copy link
Author

Choose a reason for hiding this comment

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

add parameter to avoid accessing a global variable

export default class HeatmapCalendar extends Plugin {
settings: CalendarSettings
Copy link
Author

Choose a reason for hiding this comment

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

removing this since it is not needed, and this way we can enable strictPropertyInitialization in tsconfig

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant