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

Add support for custom types to "typeorm-typescript/enforce-column-types" #5

Closed
SimonSimCity opened this issue May 21, 2024 · 9 comments

Comments

@SimonSimCity
Copy link

I'd love to get an option to define things like import { UUID } from 'crypto'; as compatible to string. This way I could take advantage of the null-checking here!

@daniel7grant
Copy link
Owner

I detailed in another comment (#4 (comment)) what is the issue with aliases. However I could imagine an option in the ESLint configuration, where you could add custom aliases:

{
  "rules": {
    "typeorm-typescript/enforce-column-types": ["error", {
      "customTypes": {
        "UUID": "string"
      }
    }]
  }
}

It would be riskier, but there could even be a built-in mapping of well-known types, that is validated automatically (for example somebody redefining UUID to store it as an object). However I'd prefer to get this first as an opt-in feature.

Would this be what you had in mind?

@SimonSimCity
Copy link
Author

This idea here could be an intermediate step towards full type support (as I suggested in my comment on #4). It's not quite type-safe, but I guess that's something I'd have to live with then 😅 My database-layer anyways won't be 100% type safe, because I cannot create a rule by which it would check for my uuid pattern, so I'll always have to rely on e.g. that my application is the only thing modifying the database.

@daniel7grant
Copy link
Owner

I added some support for type safety in v0.3.0 so it should now handle aliases (your UUID example should work as well). Thank you for your suggestion!

@norbornen
Copy link

    "typeorm-typescript/enforce-column-types": ["error", {
      "customTypes": {
        "UUID": "string"
      }
    }]

What about this?
My database driver (mysql) cast bigint and decimal to string...

@daniel7grant daniel7grant reopened this Nov 3, 2024
@daniel7grant
Copy link
Owner

Can you link me a documentation that shows that decimal should be parsed as string? I found that bigint is parsed into a string in the docs, I'll change that in the next release.

@norbornen
Copy link

norbornen commented Nov 4, 2024

This may depend on both the internal default settings of typeorm and the specific settings of the driver, and on the settings set by the developer.

Example for typeorm usage with custom bigint handler:

const pg = require('pg');

pg.types.setTypeParser(20, parseInt); // for BigInt

/** @type {import('typeorm/driver/postgres/PostgresConnectionOptions').PostgresConnectionOptions} */
const ormconfig = {
  type: "postgres",
  ...
};

module.exports = ormconfig;

@daniel7grant
Copy link
Owner

I tested Typeorm with the most popular drivers (pg, mysql2, sqlite3), and I found that you are indeed correct. It gives different outputs for different drivers:

pg mysql2 sqlite3
bigint string string number
decimal string string number

I would guess this is based on SQLite's lack of different sized int and float columns.

I'm not sure how to proceed, I'm wary of adding new configurations for every change, which would nudge me towards accepting both string and number. On the other hand, that would add to the confusion that this library is supposed to solve. Do you think a driver configuration with sqlite option and moving the default value to string would solve this problem?

@daniel7grant
Copy link
Owner

Hey @norbornen, I created a new PR #19, which changes the default behaviour to follow your use-case. Can you follow up, if this is fixed for you? I can merge it for the next release.

@daniel7grant
Copy link
Owner

I released the fix for bigint and decimal in version v0.5.0. If you have any more issues, feel free to reopen.

Thanks for your help!

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

No branches or pull requests

3 participants