ref: replace consistent-type-imports with verbatimModuleSyntax #95225
Conversation
to make it work under isolatedModules; also, this constant is only used within issueList, so it makes sense to not have it a global constant
github-actions
bot
added
the
Scope: Frontend
| import {t, tct} from 'sentry/locale'; | ||
| import type {Event} from 'sentry/types/event'; | ||
| import type {Group, GroupTombstoneHelper} from 'sentry/types/group'; | ||
| import type {Organization} from 'sentry/types/organization'; | ||
|
|
||
| export const DEFAULT_QUERY = 'is:unresolved issue.priority:[high, medium]'; | ||
|
|
||
| export enum Query { | ||
| FOR_REVIEW = 'is:unresolved is:for_review assigned_or_suggested:[me, my_teams, none]', | ||
| PRIORITIZED = DEFAULT_QUERY, // eslint-disable-line @typescript-eslint/prefer-literal-enum-member |
There was a problem hiding this comment.
note: verbatimModuleSyntax also enables isolatedModules, where files must be parseable on their own. For enums, that means they can’t reference a value outside of its file, or it’ll error with:
TS18055: 'Query.PRIORITIZED' has a string type, but must have syntactically recognizable string syntax when 'isolatedModules' is enabled.
This is the only place where we had this violation (it’s also a lint violation as we can see), and since DEFAULT_QUERY is only used within views/issueList, it makes sense to inline it here, as this also makes the constant smaller in scope / less global.
| // We have a jest.config.ts file in ESM syntax but with verbatimModuleSyntax, | ||
| // this is seen as a CommonJS file by Jest because we don't have type: "module" set in package.json. | ||
| // The separate tsconfig.jest.json file turns off verbatimModuleSyntax | ||
| process.env.TS_NODE_PROJECT = 'tsconfig.jest.json'; | ||
|
|
There was a problem hiding this comment.
this was a bit of a struggle to get there. to elaborate a bit:
Under the hood, jest uses ts-node to parse .ts config files (as it needs them to be .js), and it picks up tsconfig.json per default to do so.
With verbatimModuleSyntax being on, .ts files are being treated as commonJs - because we don’t have type: 'module' being defined in package.json.
However, the file is actually written in ESM syntax. One fix would be to rename it to .mts - the typescript equivalent of .mjs, however, jest doesn’t allow this and can’t pick it up, even when passed with --config.
The best fix would be to add type: 'module' to our package.json, however, that's a larger fix as we have .js scripts that are in commonJs, so they would need to move to .cjs or be re-written to ESM.
I’m unsure who owns this, but the split between esm/cjs and extensions we have is a bit messy
| @@ -275,7 +275,7 @@ | |||
| "validate-api-examples": "pnpm run --dir api-docs openapi-examples-validator ../tests/apidocs/openapi-derefed.json --no-additional-properties", | |||
| "mkcert-localhost": "mkcert -key-file config/localhost-key.pem -cert-file config/localhost.pem localhost 127.0.0.1 dev.getsentry.net *.dev.getsentry.net && mkcert -install", | |||
| "https-proxy": "caddy run --config - <<< '{\"apps\":{\"http\":{\"servers\":{\"srv0\":{\"listen\":[\":8003\"],\"routes\":[{\"handle\":[{\"handler\":\"reverse_proxy\",\"upstreams\":[{\"dial\":\"localhost:8000\"}]}]}],\"tls_connection_policies\":[{\"certificate_selection\":{\"any_tag\":[\"cert0\"]}}]}}},\"tls\":{\"certificates\":{\"load_files\":[{\"certificate\":\"./config/localhost.pem\",\"key\":\"./config/localhost-key.pem\",\"tags\":[\"cert0\"]}]}}}}'", | |||
| "knip": "knip --treat-config-hints-as-errors", | |||
| "knip": "TS_NODE_PROJECT=tsconfig.jest.json knip --treat-config-hints-as-errors", | |||
There was a problem hiding this comment.
note: since knip also parses the jest.config.ts file, it suffers from the same problem as jest, so I’ve applied the same “fix”.
This PR replaces typescript-eslint/consistent-type-imports with the TypeScript compiler setting verbatimModuleSyntax.
Under
verbatimModuleSyntax, types must be imported withimport type, so we get the same benefit, but using the ts compiler for this is faster than adding a type-aware lint rule.