- Notifications
You must be signed in to change notification settings - Fork530
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
feat: configuration for seamless cli use in a typescript project#1439
base:main
Are you sure you want to change the base?
Conversation
- In this commit, i have modified the templates and did other related changes to add support for typescript projects.- The new changes I made, will automatically detect if the project that uses sequelize-cli is on typescirpt or not and will switch logic based on it.
| dataValues: null, | ||
| }; | ||
| } else if (split.length === 3) { | ||
| const validValues = /^\{(,? ?[A-z0-9 ]+)+\}$/; | ||
| const validValues = | ||
| /^\{((('[A-z0-9 ]+')|("[A-z0-9 ]+")|([A-z0-9 ]+)))(, ?(('[A-z0-9 ]+')|("[A-z0-9 ]+")|([A-z0-9 ]+)))*\}$/; |
Check failure
Code scanning / CodeQL
Inefficient regular expression
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
it's a false positive,@WikiRik can you please dismiss this alert?
| dataValues: null, | ||
| }; | ||
| } else if (split.length === 3) { | ||
| const validValues = /^\{(,? ?[A-z0-9 ]+)+\}$/; | ||
| const validValues = | ||
| /^\{((('[A-z0-9 ]+')|("[A-z0-9 ]+")|([A-z0-9 ]+)))(, ?(('[A-z0-9 ]+')|("[A-z0-9 ]+")|([A-z0-9 ]+)))*\}$/; |
Check warning
Code scanning / CodeQL
Overly permissive regular expression range
| dataValues: null, | ||
| }; | ||
| } else if (split.length === 3) { | ||
| const validValues = /^\{(,? ?[A-z0-9 ]+)+\}$/; | ||
| const validValues = | ||
| /^\{((('[A-z0-9 ]+')|("[A-z0-9 ]+")|([A-z0-9 ]+)))(, ?(('[A-z0-9 ]+')|("[A-z0-9 ]+")|([A-z0-9 ]+)))*\}$/; |
Check warning
Code scanning / CodeQL
Overly permissive regular expression range
| dataValues: null, | ||
| }; | ||
| } else if (split.length === 3) { | ||
| const validValues = /^\{(,? ?[A-z0-9 ]+)+\}$/; | ||
| const validValues = | ||
| /^\{((('[A-z0-9 ]+')|("[A-z0-9 ]+")|([A-z0-9 ]+)))(, ?(('[A-z0-9 ]+')|("[A-z0-9 ]+")|([A-z0-9 ]+)))*\}$/; |
Check warning
Code scanning / CodeQL
Overly permissive regular expression range
| dataValues: null, | ||
| }; | ||
| } else if (split.length === 3) { | ||
| const validValues = /^\{(,? ?[A-z0-9 ]+)+\}$/; | ||
| const validValues = | ||
| /^\{((('[A-z0-9 ]+')|("[A-z0-9 ]+")|([A-z0-9 ]+)))(, ?(('[A-z0-9 ]+')|("[A-z0-9 ]+")|([A-z0-9 ]+)))*\}$/; |
Check warning
Code scanning / CodeQL
Overly permissive regular expression range
| dataValues: null, | ||
| }; | ||
| } else if (split.length === 3) { | ||
| const validValues = /^\{(,? ?[A-z0-9 ]+)+\}$/; | ||
| const validValues = | ||
| /^\{((('[A-z0-9 ]+')|("[A-z0-9 ]+")|([A-z0-9 ]+)))(, ?(('[A-z0-9 ]+')|("[A-z0-9 ]+")|([A-z0-9 ]+)))*\}$/; |
Check warning
Code scanning / CodeQL
Overly permissive regular expression range
| dataValues: null, | ||
| }; | ||
| } else if (split.length === 3) { | ||
| const validValues = /^\{(,? ?[A-z0-9 ]+)+\}$/; | ||
| const validValues = | ||
| /^\{((('[A-z0-9 ]+')|("[A-z0-9 ]+")|([A-z0-9 ]+)))(, ?(('[A-z0-9 ]+')|("[A-z0-9 ]+")|([A-z0-9 ]+)))*\}$/; |
Check warning
Code scanning / CodeQL
Overly permissive regular expression range
| @@ -1,8 +1,9 @@ | |||
| 'use strict'; | |||
| <%= isTypescriptProject ? `import { QueryInterface, DataTypes } from 'sequelize';` : '' %> | |||
| /** @type {import('sequelize-cli').Migration} */ | |||
| module.exports = { | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
i think also worthexport default no?
| } | ||
| } | ||
| const isTypescriptProject = isPackageInstalled('typescript'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
what about having a way to decide it from the CLI? e.g.npx sequelize-cli migration:generate --name XXX --typescript and then inside each*_generate.js provide theisTypescript to therender function?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
This is also how we are going to do it in the new CLI;https://github.com/sequelize/sequelize/blob/main/packages/cli/src/commands/migration/generate.ts
In this commit, I have modified the templates and made other related changes to add support for typescript projects.
The new changes I made, will automatically detect if the project that uses sequelize-cli is on typescript or not and will switch logic based on it.
Pull Request check-list
Please make sure to review and check all of these items:
npm run testpass with this change (including linting)?Description of change
Link to the issue:#1436
The sequelize-cli didn't have the support for typescript projects and in this PR I implemented certain changes that will make it capable of handling the CLI for both typescript and javascript files, additionally, I have improved the templates to be able to generate both
.tsand.jsfiles for migrations, models, and seeds.