Export DatabaseError, ErrorLevel, and TransactionStatus as values

[azerum]

Currently DatabaseError is exported with export type, which means that the users of the library cannot import the class of DatabaseError itself to do instanceof checks

The following (example) code breaks at runtime when compiled to either ESM or CommonJS:

import { DatabaseError } from 'ts-postgres'

try {
    throw new Error()
}
catch (error) {
    if (error instanceof DatabaseError) {
        console.log('Unexpected')
    }
}

ESM error:

import { DatabaseError } from 'ts-postgres';
         ^^^^^^^^^^^^^
SyntaxError: The requested module 'ts-postgres' does not provide an export named 'DatabaseError'

Similarly, one might want to access enums ErrorLevel and TransactionStatus, but at runtime those cannot be imported due to export type

[malthe]

I do question doing this for ErrorLevel and TransactionStatus – these are both const enum and being exported as type should not prevent you from accessing their fields and compare them to the enum values, e.g. client.transactionStatus === TransactionStatus.Idle.

This should be an integer comparison where the symbol Idle is defined only in the type definition.

[azerum]

Turns out some tools (which I use) don't play well with const enum, example is vitest testing framework, which uses vite for code building, which uses esbuild

This comment was helpful for my understanding. Essentially, TypeScript compiler looks at enums in d.ts files to decide what number to replace TransactionStatus.Idle occurrences in code. This contradicts one expectation about TypeScript: that d.ts files and types in general do not affect the emitted JS code

esbuild relies on such assumption, so it does not check d.ts files at all

Hence when running this test in vitest, the TS code is compiled to roughly this JS code:

import { TransactionStatus, connect } from "ts-postgres";
import { expect, test } from "vitest";
test("Initially, transactionStatus is Idle", async () => {
  const client = await connect({
    database: "postgres"
  });
  expect(client.transactionStatus).toBe(TransactionStatus.Idle);
});

Where TransactionStatus.Idle is not replaced (since d.ts. files are not checked). If exported as type, TransactionStatus does not exist at runtime, so we get error in the test:

TypeError: Cannot read properties of undefined (reading 'Idle')

When exporting as value

I've created a repository with two commits: one uses ts-postgres version which exports TransactionStatus as type, and other - which exports it as value. I've compiled code with tsc, esbuild, and run a vitest test and compared behavior

Here's the repository: https://github.com/azerum/ts-postgres-const-enum

You can see the README.md for results (tsc/esbuild output, esbuild runtime behavior, vitest output)

In short, when exporting TransactionStatus as value, TypeScript generates this code in dist/module/protocol.js:

export var TransactionStatus;
(function (TransactionStatus) {
    TransactionStatus[TransactionStatus["Idle"] = 73] = "Idle";
    TransactionStatus[TransactionStatus["InTransaction"] = 84] = "InTransaction";
    TransactionStatus[TransactionStatus["InError"] = 69] = "InError";
})(TransactionStatus || (TransactionStatus = {}));

This happens because you have isolatedModules: true in your tsconfig.json, and it implies default value preserveConstEnums: true (https://www.typescriptlang.org/tsconfig#preserveConstEnums)

This allows vitest test to pass and esbuild-built code to run. Yet, when the user of the library compiles code with regular tsc, inlining is still applied (see npm run build:tsc in the readme)

The only disadvantage of const-enum + preserveConstEnums I see is increased emitted code size. I thing it's a better trade-off to support widely used vitest, vite, esbuild

[malthe]

The change to const enum was actually motivated by #100 – but it had not occurred to me that they're actually against that core tenant of TypeScript, not relying on type-directed emit.

I think the solution is to drop the use of enums as a public interface since they're inefficient without their const enum variant in the sense that they unnecessarily contribute to code bloat.

It would seem that a simple constant assignment such as TransactionStatusIdle = 0x49 is preferable, instead letting TypeScript help with a fixed set of valid constants for a particular value:

type TransactionStatus = TransactionStatusIdle | TransactionStatusInTransaction | ...

A stricter solution would be the use of symbols I suppose, but this seems unnecessary to me.

Thanks for taking the time to explain this behavior in such detail.

[azerum]

Thanks for sharing the idea

I don't use enums often, and the issue #100 does not specify concrete examples of when enums are inefficient. Are they problematic because of the increased emitted code size? Or because they compile to functions that construct an object with potentially many keys? Have enums ever caused issues in practice?

The solution with constants would be along a lines of this?

export const TransactionStatusIdle = 0x49
export const TransactionStatusInTransaction = 0x54
export const TransactionStatusInError = 0x45

export type TransactionStatus = 
    | typeof TransactionStatusIdle
    | typeof TransactionStatusInTransaction
    | typeof TransactionStatusInError

Given the TransactionStatus- prefix, the constants are unlikely to clash. The users could also inspect the client.transactionStatus property to see its TS type. One can also discover possible values via autocomplete. So it seems fine

It might be inconvenient to convert large enum, such as DataType, to such syntax, but writing convenience is perhaps not the most important criteria

[malthe]

Yes, I think the point is exactly that the inconvenience isn't so bad, and the detriment in using enums is significant, in terms of code size and misalignment with a core TypeScript tenant.