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

Update eslint config and enable shopify ts rules #18

Merged
merged 10 commits into from
Oct 2, 2024
Merged

Conversation

wheelsandcogs
Copy link
Collaborator

@wheelsandcogs wheelsandcogs commented Sep 19, 2024

This updates the eslint config to use the newer 'flat' config style, turns on the shopify typescript rules and fixes any issues resulting from that.

It resolves the issue we were having with eslint not liking enums.

I've disabled @typescript-eslint/member-ordering as it would involve reordering all the static methods in most of the DTOs and ain't nobody got time for that.

'error',
{
selector: 'default',
format: ['camelCase', 'PascalCase', 'UPPER_CASE', 'snake_case'],
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

default seems to be ['camelCase', 'PascalCase', 'UPPER_CASE'] which gives us a lot of errors due to the DTOs.

@wheelsandcogs wheelsandcogs changed the title WIP linting Update eslint config to use "flat-config" and enable shopify ts rules Sep 27, 2024
@wheelsandcogs wheelsandcogs changed the title Update eslint config to use "flat-config" and enable shopify ts rules Update eslint config and enable shopify ts rules Sep 27, 2024
@@ -1,9 +1,7 @@
/* eslint-disable no-shadow */
/* eslint-disable no-unused-vars */
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Enums work fine now without these.

Copy link
Collaborator Author

@wheelsandcogs wheelsandcogs left a comment

Choose a reason for hiding this comment

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

This is a fairly dull PR to bring consistency to our naming of things.

Ci = 'ci',
Staging = 'staging',
Prod = 'production',
Default = 'default'
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Shopify specifies PascalCase for enum keys for some reason so might as well stick with that. I've also converted all enum values to snake_case for consistency.

@@ -42,7 +42,7 @@ function hashReadableStream(stream: Readable, algorithm: string = 'sha256'): Pro
});
}

function paginate<T>(array: Array<T>, page_number: number, page_size: number): Array<T> {
function paginate<T>(array: T[], page_number: number, page_size: number): T[] {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

One of the shopify rules is use [] instead of Array for types.

@@ -33,7 +33,7 @@ export class DatasetDTO {
static async fromDatasetShallow(dataset: Dataset): Promise<DatasetDTO> {
const dto = new DatasetDTO();
dto.id = dataset.id;
dto.creation_date = dataset.creationDate.toISOString();
dto.created_at = dataset.createdAt.toISOString();
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

all timestamp fields are now xxx_at

@@ -22,7 +19,7 @@ export class Dimension extends BaseEntity {
@JoinColumn({ name: 'dataset_id', foreignKeyConstraintName: 'FK_dimension_dataset_id' })
dataset: Promise<Dataset>;

@Column({ type: 'enum', enum: Object.keys(DimensionType), nullable: false })
@Column({ type: 'enum', enum: Object.values(DimensionType), nullable: false })
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Everywhere else we were using the enum values rather than enum keys so changed this for consistency.

Copy link
Collaborator

@j-maynard j-maynard left a comment

Choose a reason for hiding this comment

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

LGTM 🚀

@wheelsandcogs wheelsandcogs merged commit 0d10f45 into main Oct 2, 2024
3 checks passed
@wheelsandcogs wheelsandcogs deleted the fix/linting branch October 2, 2024 08:16
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.

2 participants