Skip to content

ref: replace consistent-type-imports with verbatimModuleSyntax #95225

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

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions config/tsconfig.base.json
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@
"esModuleInterop": true,
"experimentalDecorators": true,
"resolveJsonModule": true,
"verbatimModuleSyntax": true,

"paths": {
"sentry/*": ["../static/app/*"],
Expand Down
1 change: 0 additions & 1 deletion eslint.config.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -480,7 +480,6 @@ export default typescript.config([
? {
'@typescript-eslint/await-thenable': 'error',
'@typescript-eslint/consistent-type-exports': 'error',
'@typescript-eslint/consistent-type-imports': 'error',
'@typescript-eslint/no-array-delete': 'error',
'@typescript-eslint/no-base-to-string': 'error',
'@typescript-eslint/no-for-in-array': 'error',
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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”.

"knip:prod": "pnpm run knip --production --exclude exports,types,dependencies,unresolved"
},
"browserslist": {
Expand Down
5 changes: 5 additions & 0 deletions scripts/test.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,11 @@ process.env.NODE_ENV = 'test';
process.env.PUBLIC_URL = '';
process.env.TZ = 'America/New_York';

// 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';

Comment on lines +9 to +13
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

// Makes the script crash on unhandled rejections instead of silently
// ignoring them. In the future, promise rejections that are not handled will
// terminate the Node.js process with a non-zero exit code.
Expand Down
1 change: 0 additions & 1 deletion static/app/constants/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -234,7 +234,6 @@ export const MAX_PICKABLE_DAYS = 90;

export const DEFAULT_STATS_PERIOD = '14d';

export const DEFAULT_QUERY = 'is:unresolved issue.priority:[high, medium]';
export const TAXONOMY_DEFAULT_QUERY = 'is:unresolved';

export const DEFAULT_USE_UTC = true;
Expand Down
2 changes: 1 addition & 1 deletion static/app/views/issueList/actions/index.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,12 @@ import {
} from 'sentry-test/reactTestingLibrary';

import GlobalModal from 'sentry/components/globalModal';
import {DEFAULT_QUERY} from 'sentry/constants';
import GroupStore from 'sentry/stores/groupStore';
import SelectedGroupStore from 'sentry/stores/selectedGroupStore';
import {IssueCategory} from 'sentry/types/group';
import * as analytics from 'sentry/utils/analytics';
import {IssueListActions} from 'sentry/views/issueList/actions';
import {DEFAULT_QUERY} from 'sentry/views/issueList/utils';

const organization = OrganizationFixture();

Expand Down
3 changes: 1 addition & 2 deletions static/app/views/issueList/issueListSetAsDefault.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,9 @@ import {SearchFixture} from 'sentry-fixture/search';
import {initializeOrg} from 'sentry-test/initializeOrg';
import {render, screen, userEvent, waitFor} from 'sentry-test/reactTestingLibrary';

import {DEFAULT_QUERY} from 'sentry/constants';
import {SavedSearchType, SavedSearchVisibility} from 'sentry/types/group';
import IssueListSetAsDefault from 'sentry/views/issueList/issueListSetAsDefault';
import {IssueSortOptions} from 'sentry/views/issueList/utils';
import {DEFAULT_QUERY, IssueSortOptions} from 'sentry/views/issueList/utils';

describe('IssueListSetAsDefault', () => {
const organization = OrganizationFixture();
Expand Down
3 changes: 1 addition & 2 deletions static/app/views/issueList/noGroupsHandler/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,11 @@ import type {Client} from 'sentry/api';
import EmptyStateWarning from 'sentry/components/emptyStateWarning';
import LoadingIndicator from 'sentry/components/loadingIndicator';
import Placeholder from 'sentry/components/placeholder';
import {DEFAULT_QUERY} from 'sentry/constants';
import {t} from 'sentry/locale';
import type {Organization} from 'sentry/types/organization';
import type {Project} from 'sentry/types/project';
import NoIssuesMatched from 'sentry/views/issueList/noGroupsHandler/noIssuesMatched';
import {FOR_REVIEW_QUERIES} from 'sentry/views/issueList/utils';
import {DEFAULT_QUERY, FOR_REVIEW_QUERIES} from 'sentry/views/issueList/utils';

import NoUnresolvedIssues from './noUnresolvedIssues';

Expand Down
2 changes: 1 addition & 1 deletion static/app/views/issueList/overview.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ import {
} from 'sentry-test/reactTestingLibrary';
import {textWithMarkupMatcher} from 'sentry-test/utils';

import {DEFAULT_QUERY} from 'sentry/constants';
import PageFiltersStore from 'sentry/stores/pageFiltersStore';
import ProjectsStore from 'sentry/stores/projectsStore';
import TagStore from 'sentry/stores/tagStore';
Expand All @@ -28,6 +27,7 @@ import type {RouteComponentProps} from 'sentry/types/legacyReactRouter';
import localStorageWrapper from 'sentry/utils/localStorage';
import * as parseLinkHeader from 'sentry/utils/parseLinkHeader';
import IssueListOverview from 'sentry/views/issueList/overview';
import {DEFAULT_QUERY} from 'sentry/views/issueList/utils';

const DEFAULT_LINKS_HEADER =
'<http://127.0.0.1:8000/api/0/organizations/org-slug/issues/?cursor=1443575731:0:1>; rel="previous"; results="false"; cursor="1443575731:0:1", ' +
Expand Down
4 changes: 2 additions & 2 deletions static/app/views/issueList/overview.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import * as Layout from 'sentry/components/layouts/thirds';
import {extractSelectionParameters} from 'sentry/components/organizations/pageFilters/utils';
import type {CursorHandler} from 'sentry/components/pagination';
import QueryCount from 'sentry/components/queryCount';
import {DEFAULT_QUERY, DEFAULT_STATS_PERIOD} from 'sentry/constants';
import {DEFAULT_STATS_PERIOD} from 'sentry/constants';
import {t, tct} from 'sentry/locale';
import GroupStore from 'sentry/stores/groupStore';
import IssueListCacheStore from 'sentry/stores/IssueListCacheStore';
Expand Down Expand Up @@ -59,7 +59,7 @@ import {usePrefersStackedNav} from 'sentry/views/nav/usePrefersStackedNav';

import IssueListFilters from './filters';
import IssueListHeader from './header';
import type {QueryCounts} from './utils';
import {DEFAULT_QUERY, type QueryCounts} from './utils';
import {
DEFAULT_ISSUE_STREAM_SORT,
FOR_REVIEW_QUERIES,
Expand Down
4 changes: 3 additions & 1 deletion static/app/views/issueList/overviewWrapper.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import {DEFAULT_QUERY, TAXONOMY_DEFAULT_QUERY} from 'sentry/constants';
import {TAXONOMY_DEFAULT_QUERY} from 'sentry/constants';
import {t} from 'sentry/locale';
import type {RouteComponentProps} from 'sentry/types/legacyReactRouter';
import {defined} from 'sentry/utils';
Expand All @@ -7,6 +7,8 @@ import IssueListContainer from 'sentry/views/issueList';
import IssueListOverview from 'sentry/views/issueList/overview';
import {usePrefersStackedNav} from 'sentry/views/nav/usePrefersStackedNav';

import {DEFAULT_QUERY} from './utils';

type OverviewWrapperProps = RouteComponentProps<
Record<PropertyKey, string | undefined>,
{searchId?: string}
Expand Down
3 changes: 2 additions & 1 deletion static/app/views/issueList/utils.tsx
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
import type {Location, LocationDescriptorObject} from 'history';

import ExternalLink from 'sentry/components/links/externalLink';
import {DEFAULT_QUERY} from 'sentry/constants';
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
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Expand Down
6 changes: 6 additions & 0 deletions tsconfig.jest.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"extends": "./tsconfig.json",
"compilerOptions": {
"verbatimModuleSyntax": false
},
}
Loading