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.

Sign up for GitHub

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

Jump to bottom

Fix/eslint newline error in model generator #2812

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Fix/eslint newline error in model generator #2812

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
87fa2a8
fix: add ending newline to model generator
coolsoftwaretyler Oct 17, 2024
148e526
fix: add ending newline to model test generator
coolsoftwaretyler Oct 17, 2024
cb78066
chore: disable prettier in Jest
coolsoftwaretyler Oct 17, 2024
8984194
test: update generator snapshots
coolsoftwaretyler Oct 17, 2024
ec682f8
fix: optionally use 2 versions of prettier for Jest
coolsoftwaretyler Oct 18, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion boilerplate/ignite/templates/model/NAME.test.ts.ejs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,4 +9,4 @@ test("can be created", () => {
expect(instance).toBeTruthy()
})

// @mst remove-file
// @mst remove-file
2 changes: 1 addition & 1 deletion boilerplate/ignite/templates/model/NAME.ts.ejs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,4 +31,4 @@ export interface <%= props.pascalCaseName %>SnapshotOut extends SnapshotOut<type
export interface <%= props.pascalCaseName %>SnapshotIn extends SnapshotIn<typeof <%= props.pascalCaseName %>Model> {}
export const create<%= props.pascalCaseName %>DefaultModel = () => types.optional(<%= props.pascalCaseName %>Model, {})

// @mst remove-file
// @mst remove-file
4 changes: 3 additions & 1 deletion package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@
"husky": "^7.0.1",
"jest": "^27.0.6",
"prettier": "^3.3.3",
"prettier-2": "npm:[email protected]",
"semantic-release": "^17.4.2",
"ts-jest": "^27.0.4",
"ts-node": "^10.2.0",
Expand Down Expand Up @@ -159,6 +160,7 @@
"tsx",
"js",
"jsx"
]
],
"prettierPath": "<rootDir>/node_modules/prettier-2/index.js"
}
}
108 changes: 56 additions & 52 deletions test/vanilla/ignite-generate.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,37 +57,39 @@ describe("ignite-cli generate", () => {
"
`)
expect(read(`${TEMP_DIR}/app/models/Pizza.ts`)).toMatchInlineSnapshot(`
"import { Instance, SnapshotIn, SnapshotOut, types } from \\"mobx-state-tree\\"
import { withSetPropAction } from \\"./helpers/withSetPropAction\\"
"import { Instance, SnapshotIn, SnapshotOut, types } from \\"mobx-state-tree\\"
import { withSetPropAction } from \\"./helpers/withSetPropAction\\"

Comment on lines -60 to 62
Copy link
Contributor Author

@coolsoftwaretyler coolsoftwaretyler Oct 17, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I would guess this whitespace diff (and a few below) came from disabling Prettier in Jest, so maybe we should find a different way to fix that, or else it will cause a lot more noisy churn after the fact with other inline snapshots.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Another approach would be ec682f8, which uses the workaround of a separate Prettier version for Jest. I set prettier-2 to be npm:[email protected] which I saw was the last version before the update in https://github.com/infinitered/ignite/pull/2761/files#diff-7ae45ad102eab3b6d7e7896acd08c427a9b25b346470d7bc6507b6481575d519.

Didn't seem to change the whitespace diff here, so maybe there's no real issue. Figured I'd at least provide the commit and we could easily drop it if the prettierPath: null is acceptable.

/**
* Model description here for TypeScript hints.
*/
export const PizzaModel = types
.model(\\"Pizza\\")
.props({})
.actions(withSetPropAction)
.views((self) => ({})) // eslint-disable-line @typescript-eslint/no-unused-vars
.actions((self) => ({})) // eslint-disable-line @typescript-eslint/no-unused-vars

export interface Pizza extends Instance<typeof PizzaModel> {}
export interface PizzaSnapshotOut extends SnapshotOut<typeof PizzaModel> {}
export interface PizzaSnapshotIn extends SnapshotIn<typeof PizzaModel> {}
export const createPizzaDefaultModel = () => types.optional(PizzaModel, {})

// @mst remove-file"
`)
/**
* Model description here for TypeScript hints.
*/
export const PizzaModel = types
.model(\\"Pizza\\")
.props({})
.actions(withSetPropAction)
.views((self) => ({})) // eslint-disable-line @typescript-eslint/no-unused-vars
.actions((self) => ({})) // eslint-disable-line @typescript-eslint/no-unused-vars

export interface Pizza extends Instance<typeof PizzaModel> {}
export interface PizzaSnapshotOut extends SnapshotOut<typeof PizzaModel> {}
export interface PizzaSnapshotIn extends SnapshotIn<typeof PizzaModel> {}
export const createPizzaDefaultModel = () => types.optional(PizzaModel, {})

// @mst remove-file
"
Comment on lines +78 to +79
Copy link
Contributor Author

@coolsoftwaretyler coolsoftwaretyler Oct 17, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

This is the key snapshot diff that we do want to preserve: newline at end of file

`)
expect(read(`${TEMP_DIR}/app/models/Pizza.test.ts`)).toMatchInlineSnapshot(`
"import { PizzaModel } from \\"./Pizza\\"
"import { PizzaModel } from \\"./Pizza\\"

test(\\"can be created\\", () => {
const instance = PizzaModel.create({})
test(\\"can be created\\", () => {
const instance = PizzaModel.create({})

expect(instance).toBeTruthy()
})
expect(instance).toBeTruthy()
})

// @mst remove-file"
`)
// @mst remove-file
"
`)
expect(read(`${TEMP_DIR}/app/models/index.ts`)).toMatchInlineSnapshot(`
"export * from \\"./RootStore\\"
export * from \\"./helpers/getRootStore\\"
Expand Down Expand Up @@ -138,37 +140,39 @@ describe("ignite-cli generate", () => {
"
`)
expect(read(`${TEMP_DIR}/app/models/PizzaStore.ts`)).toMatchInlineSnapshot(`
"import { Instance, SnapshotIn, SnapshotOut, types } from \\"mobx-state-tree\\"
import { withSetPropAction } from \\"./helpers/withSetPropAction\\"
"import { Instance, SnapshotIn, SnapshotOut, types } from \\"mobx-state-tree\\"
import { withSetPropAction } from \\"./helpers/withSetPropAction\\"

/**
* Model description here for TypeScript hints.
*/
export const PizzaStoreModel = types
.model(\\"PizzaStore\\")
.props({})
.actions(withSetPropAction)
.views((self) => ({})) // eslint-disable-line @typescript-eslint/no-unused-vars
.actions((self) => ({})) // eslint-disable-line @typescript-eslint/no-unused-vars

export interface PizzaStore extends Instance<typeof PizzaStoreModel> {}
export interface PizzaStoreSnapshotOut extends SnapshotOut<typeof PizzaStoreModel> {}
export interface PizzaStoreSnapshotIn extends SnapshotIn<typeof PizzaStoreModel> {}
export const createPizzaStoreDefaultModel = () => types.optional(PizzaStoreModel, {})

// @mst remove-file"
`)
/**
* Model description here for TypeScript hints.
*/
export const PizzaStoreModel = types
.model(\\"PizzaStore\\")
.props({})
.actions(withSetPropAction)
.views((self) => ({})) // eslint-disable-line @typescript-eslint/no-unused-vars
.actions((self) => ({})) // eslint-disable-line @typescript-eslint/no-unused-vars

export interface PizzaStore extends Instance<typeof PizzaStoreModel> {}
export interface PizzaStoreSnapshotOut extends SnapshotOut<typeof PizzaStoreModel> {}
export interface PizzaStoreSnapshotIn extends SnapshotIn<typeof PizzaStoreModel> {}
export const createPizzaStoreDefaultModel = () => types.optional(PizzaStoreModel, {})

// @mst remove-file
"
`)
expect(read(`${TEMP_DIR}/app/models/PizzaStore.test.ts`)).toMatchInlineSnapshot(`
"import { PizzaStoreModel } from \\"./PizzaStore\\"
"import { PizzaStoreModel } from \\"./PizzaStore\\"

test(\\"can be created\\", () => {
const instance = PizzaStoreModel.create({})
test(\\"can be created\\", () => {
const instance = PizzaStoreModel.create({})

expect(instance).toBeTruthy()
})
expect(instance).toBeTruthy()
})

// @mst remove-file"
`)
// @mst remove-file
"
`)
expect(read(`${TEMP_DIR}/app/models/index.ts`)).toMatchInlineSnapshot(`
"export * from \\"./RootStore\\"
export * from \\"./helpers/getRootStore\\"
Expand Down
5 changes: 5 additions & 0 deletions yarn.lock
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5718,6 +5718,11 @@ prelude-ls@^1.2.1:
resolved "https://registry.yarnpkg.com/prelude-ls/-/prelude-ls-1.2.1.tgz#debc6489d7a6e6b0e7611888cec880337d316396"
integrity sha512-vkcDPrRZo1QZLbn5RLGPpg/WmIQ65qoWWhcGKf/b5eplkkarX0m9z8ppCat4mlOqUsWpyNuYgO3VRyrYHSzX5g==

"prettier-2@npm:[email protected]":
version "2.8.8"
resolved "https://registry.yarnpkg.com/prettier/-/prettier-2.8.8.tgz#e8c5d7e98a4305ffe3de2e1fc4aca1a71c28b1da"
integrity sha512-tdN8qQGvNjw4CHbY+XXk0JgCXn9QiF21a55rBe5LJAU+kDyC4WQn4+awm2Xfk2lQMk5fKup9XgzTZtGkjBdP9Q==

prettier@^3.3.3:
version "3.3.3"
resolved "https://registry.yarnpkg.com/prettier/-/prettier-3.3.3.tgz#30c54fe0be0d8d12e6ae61dbb10109ea00d53105"
Expand Down