romeerez/orchid-orm

Narrowing column type with asType example outdated

Closed this issue · 9 comments

asType example from the documentation is no more working:

export class Table extends BaseTable {
  readonly table = 'table';
  columns = this.setColumns((t) => ({
    size: t.text().asType((t) => t<'small' | 'medium' | 'large'>()),
  }));
}
image

with min/max filled, this gives:

image
  • orchid-orm 1.21.6

I caught a flu and need a few more days to get well I guess.

Sorry for making such bugs, but that change was really important and will allow to integrate move validation libraries in a better way in not so distant future.

Sure, absolutely. No complain intended at all, this was just a heads up / documentation TODO. Get well soon!

I wanted to come up with a fix (after all, asType was partly my baby :) but I saw too many test were broken / mistyped. Even basic things like User exported from packages/qb/pqb/src/test-utils/test-utils.ts are broken due to t.json() and ...t.timestamps().

Also, I'm seeing a lot of errors like this:

pqb:check: FAIL src/columns/dateTime.test.ts
pqb:check:   ● Test suite failed to run
pqb:check:
pqb:check:     TypeError: Converting circular structure to JSON
pqb:check:         --> starting at object with constructor 'Db'
pqb:check:         --- property 'queryBuilder' closes the circle
pqb:check:         at stringify (<anonymous>)
pqb:check:
pqb:check:       at messageParent (../../../../node_modules/.pnpm/jest-worker@29.5.0/node_modules/jest-worker/build/workers/messageParent.js:29:19)

Am I missing something, or is it work in progress where the code base ran ahead of the tests suite?

By the way, perhaps it's time to adopt semver more closely? From the looks of it, these are major changes which IMO deserved a breaking change 3.0 (or rather 3.0-alpha at the current state) release.

Am I missing something, or is it work in progress where the code base ran ahead of the tests suite?

Hm, so you're saying that tests doesn't pass for freshly fetched codebase?

Packages are interlinked with ts paths aliases and in some cases it can act weirdly. I'm not 100% sure how this works, but when it seems there is something wrong with how one package import another, I run pnpm build so all are been build, and works ok. This happens rarely and most of the time it just works.

Converting circular structure to JSON

This runtime error happened and Jest can't present it, in such cases I wrap the code in the test into try/catch and log error - log can log anything. What's wrong with this error? Db errors contain a query object - deeply nested structure with some recursive links in it, I thought it may be a good idea to attach a query object db error for debugging. So far, it didn't really help but confuse Jest logger.

Fix is on its way, the fix itself was straightforward, but I started some optimizations along the way and it went out of control, so will be a big PR with a small fix and tons of optimizations.

By the way, perhaps it's time to adopt semver more closely? From the looks of it, these are major changes which IMO deserved a breaking change 3.0 (or rather 3.0-alpha at the current state) release.

Well. It shouldn't have to break anything. If you don't use zod schema, here is a bug in asType that wasn't intended, and other than that it should have just worked. And if you do use zod schema, I realized that implementation had a little sense, because it didn't have separate input and output schemas, it's pretty useless for practical use, so this is more like a fix for already existing problem rather than a breaking change of previously stable functionality.

Hm, so you're saying that tests doesn't pass for freshly fetched codebase?

Yes, exactly so. See the log:

⏺ tmp % git clone git@github.com:romeerez/orchid-orm.git
Cloning into 'orchid-orm'...
remote: Enumerating objects: 22101, done.
remote: Counting objects: 100% (5646/5646), done.
remote: Compressing objects: 100% (1603/1603), done.
remote: Total 22101 (delta 4086), reused 5403 (delta 4017), pack-reused 16455
Receiving objects: 100% (22101/22101), 5.37 MiB | 4.82 MiB/s, done.
Resolving deltas: 100% (16817/16817), done.
⏺ tmp % cd orchid-orm
⏺ tmp/orchid-orm (main) % echo 18 > .node-version
⏺ tmp/orchid-orm (main) % pnpm i
Scope: all 10 workspace projects
Lockfile is up to date, resolution step is skipped
Packages: +620
++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
Progress: resolved 620, reused 620, downloaded 0, added 620, done

devDependencies:
+ @changesets/cli 2.26.1
+ @swc/core 1.3.46
+ @swc/helpers 0.5.0
+ @swc/jest 0.2.24
+ @types/jest 29.5.0
+ @types/node 18.15.11
+ @types/pg 8.10.2
+ @typescript-eslint/eslint-plugin 5.57.1
+ @typescript-eslint/parser 5.57.1
+ dotenv 16.0.3
+ esbuild 0.17.15
+ eslint 8.37.0
+ eslint-config-prettier 8.8.0
+ eslint-plugin-prettier 4.2.1
+ jest 29.5.0
+ pg 8.11.2
+ prettier 2.8.7
+ rimraf 4.4.1
+ rollup 3.20.2
+ rollup-plugin-dts 5.3.0
+ rollup-plugin-esbuild 5.0.0
+ ts-node 10.9.1
+ tslib 2.5.0
+ turbo 1.8.8
+ typescript 5.0.3

Done in 2.3s
⏺ tmp/orchid-orm (main) % pnpm pqb check

> @0.0.0 pqb /Users/is/tmp/orchid-orm
> pnpm --filter pqb "check"


> pqb@0.23.5 check /Users/is/tmp/orchid-orm/packages/qb/pqb
> jest

 PASS  src/columns/array.test.ts
 PASS  src/queryMethods/rawSql.test.ts
 PASS  src/columns/operators.test.ts
 PASS  src/modules/copyTableData.test.ts
 PASS  src/columns/number.test.ts
 PASS  src/queryMethods/merge.test.ts
 FAIL  src/modules/getColumnInfo.test.ts
  ● columnInfo › should return all columns info

...

 PASS  src/common/utils.test.ts
 PASS  src/columns/enum.test.ts
 PASS  src/columns/boolean.test.ts
 FAIL  src/queryMethods/upsertOrCreate.test.ts
  ● Test suite failed to run

    TypeError: Converting circular structure to JSON
        --> starting at object with constructor 'Db'
        --- property 'queryBuilder' closes the circle
        at stringify (<anonymous>)

      at messageParent (../../../../node_modules/.pnpm/jest-worker@29.5.0/node_modules/jest-worker/build/workers/messageParent.js:29:19)

 FAIL  src/queryMethods/join/joinLateral.test.ts
  ● Test suite failed to run

    TypeError: Converting circular structure to JSON
        --> starting at object with constructor 'Db'
        --- property 'queryBuilder' closes the circle
        at stringify (<anonymous>)

      at messageParent (../../../../node_modules/.pnpm/jest-worker@29.5.0/node_modules/jest-worker/build/workers/messageParent.js:29:19)

...

  ● then › should handle .then callback properly

    expect(received).toBe(expected) // Object.is equality

    Expected: true
    Received: false

      43 |     assertType<typeof len, number>();
      44 |
    > 45 |     expect(isThenCalled).toBe(true);
         |                          ^
      46 |     expect(len).toBe(0);
      47 |   });
      48 | });

      at Object.toBe (queryMethods/then.test.ts:45:26)


Test Suites: 23 failed, 26 passed, 49 total
Tests:       5 failed, 942 passed, 947 total
Snapshots:   0 total
Time:        6.156 s
Ran all test suites.
/Users/is/tmp/orchid-orm/packages/qb/pqb:
 ERR_PNPM_RECURSIVE_RUN_FIRST_FAIL  pqb@0.23.5 check: `jest`
Exit status 1
 ELIFECYCLE  Command failed with exit code 1.
  • node v18.16.0
  • pnpm 8.15.3
  • postgres 15.5

Looks like you have skipped db setup steps (pnpm db create and pnpm db migrate in CONTRIBUTING).

I finally published a fix (with optimizations), let me know if that works for you.

Looks like you have skipped db setup steps

My bad, I was too accustomed to using empty database with migrations running inside testTransaction in my tests, so I somehow thought that would be the cases with Orchid's own tests.

I confirm that everything works now, meaning both Orchid tests and asType user-land overrides. Thanks so much.