Skip to content

Commit 22e3728

Browse files
Merge pull request #706 from reactome/spec016/t016-one-description-v2
Spec 016 T016, T017: one description per answer, and renames that fail both sides
2 parents d6c00da + a048c2f commit 22e3728

3 files changed

Lines changed: 239 additions & 3 deletions

File tree

‎e2e/content-pages.spec.ts‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -143,7 +143,9 @@ test.describe('Content pages render backend data', () => {
143143
await page.goto('/community/icon-lib/R-ICO-012449');
144144
await expect(page.locator('.detail-stid')).toHaveText('R-ICO-012449', { timeout: LOAD });
145145
await expect(page.locator('.detail-summation')).not.toBeEmpty();
146-
await expect(page.locator('.detail-preview img')).toBeVisible();
146+
// The image's address, not its loading: it comes from the release's
147+
// files, which the replay does not serve to an <img>.
148+
await expect(page.locator('.detail-preview img')).toHaveAttribute('src', /R-ICO-012449/);
147149
});
148150

149151
test('a CMS article page renders its body', async ({ page }) => {
Lines changed: 231 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,231 @@
1+
// @vitest-environment node
2+
//
3+
// One description per answer (spec 016, FR-003): an answer in the contract is
4+
// not described again by the site or the pathway browser, under its own name
5+
// or under the name a reader re-exports it as. An alias of the contract's type
6+
// is a name, not a description, and is allowed.
7+
import { readFileSync, readdirSync } from 'node:fs';
8+
import path from 'node:path';
9+
import { fileURLToPath } from 'node:url';
10+
import ts from 'typescript';
11+
import { describe, expect, it } from 'vitest';
12+
13+
const ROOT = path.resolve(path.dirname(fileURLToPath(import.meta.url)), '../..');
14+
const CONTRACT = path.join(ROOT, 'projects/contract');
15+
const READERS = ['projects/website-angular/src', 'projects/pathway-browser/src'];
16+
17+
/** Every non-spec TypeScript module under a directory, by repository path. */
18+
function modules(dir: string): Record<string, string> {
19+
const found: Record<string, string> = {};
20+
for (const file of readdirSync(path.join(ROOT, dir), { recursive: true, encoding: 'utf8' })) {
21+
if (!file.endsWith('.ts') || file.endsWith('.spec.ts') || file.endsWith('.d.ts')) continue;
22+
const where = path.posix.join(dir, file.split(path.sep).join('/'));
23+
found[where] = readFileSync(path.join(ROOT, where), 'utf8');
24+
}
25+
return found;
26+
}
27+
28+
const parse = (name: string, source: string) =>
29+
ts.createSourceFile(name, source, ts.ScriptTarget.Latest, true);
30+
31+
/** The contract's exported interfaces and types. */
32+
function contractNames(contract: Record<string, string>): Set<string> {
33+
const names = new Set<string>();
34+
for (const [file, source] of Object.entries(contract))
35+
for (const statement of parse(file, source).statements)
36+
if (
37+
(ts.isInterfaceDeclaration(statement) || ts.isTypeAliasDeclaration(statement)) &&
38+
statement.modifiers?.some((modifier) => modifier.kind === ts.SyntaxKind.ExportKeyword)
39+
)
40+
names.add(statement.name.text);
41+
return names;
42+
}
43+
44+
/** The names readers re-export the contract's types as (`export type { A as B } from '…/contract/…'`). */
45+
function readerAliases(readers: Record<string, string>): Set<string> {
46+
const names = new Set<string>();
47+
for (const [file, source] of Object.entries(readers))
48+
for (const statement of parse(file, source).statements)
49+
if (
50+
ts.isExportDeclaration(statement) &&
51+
statement.moduleSpecifier &&
52+
ts.isStringLiteral(statement.moduleSpecifier) &&
53+
/\/contract\/[^/]+\.ts$/.test(statement.moduleSpecifier.text) &&
54+
statement.exportClause &&
55+
ts.isNamedExports(statement.exportClause)
56+
)
57+
for (const element of statement.exportClause.elements) names.add(element.name.text);
58+
return names;
59+
}
60+
61+
/**
62+
* A type that describes an answer's shape itself, rather than naming another
63+
* type: an object type, a mapped type, or either inside a union, an
64+
* intersection, parentheses, an array or a wrapper (`Readonly<{…}>`).
65+
*/
66+
const describesShape = (type: ts.TypeNode): boolean =>
67+
ts.isTypeLiteralNode(type) ||
68+
ts.isMappedTypeNode(type) ||
69+
(ts.isParenthesizedTypeNode(type) && describesShape(type.type)) ||
70+
(ts.isArrayTypeNode(type) && describesShape(type.elementType)) ||
71+
(ts.isUnionTypeNode(type) && type.types.some(describesShape)) ||
72+
(ts.isIntersectionTypeNode(type) && type.types.some(describesShape)) ||
73+
(ts.isTypeReferenceNode(type) && (type.typeArguments ?? []).some(describesShape));
74+
75+
/** A declaration's name with the namespaces it sits in: `Search.Icon.Entry`. */
76+
function qualifiedName(node: ts.Node & { name: ts.Identifier }): string {
77+
const parts = [node.name.text];
78+
// A source file's parent is undefined at run time, whatever its type says.
79+
for (
80+
let at = node.parent as ts.Node | undefined;
81+
at !== undefined;
82+
at = at.parent as ts.Node | undefined
83+
)
84+
if (ts.isModuleDeclaration(at)) parts.unshift(at.name.text);
85+
return parts.join('.');
86+
}
87+
88+
/** The name a type reference names, its last part: `GroupedResult` of `GroupedResult<E>`. */
89+
const referenced = (type: ts.TypeNode): string | null =>
90+
ts.isTypeReferenceNode(type)
91+
? ts.isIdentifier(type.typeName)
92+
? type.typeName.text
93+
: type.typeName.right.text
94+
: null;
95+
96+
/**
97+
* The names readers give the contract's types by aliasing them in their own
98+
* code (`type SearchResult = GroupedResult<…>`, the pathway browser's
99+
* `Search.Entry`), by qualified name, following aliases of aliases. A type
100+
* imported from the contract under another name counts as the contract's.
101+
*/
102+
function localAliases(readers: Record<string, string>, contract: Set<string>): Set<string> {
103+
const known = new Set(contract);
104+
for (const [file, source] of Object.entries(readers))
105+
for (const statement of parse(file, source).statements)
106+
if (
107+
ts.isImportDeclaration(statement) &&
108+
ts.isStringLiteral(statement.moduleSpecifier) &&
109+
/\/contract\/[^/]+\.ts$/.test(statement.moduleSpecifier.text)
110+
) {
111+
const bindings = statement.importClause?.namedBindings;
112+
if (bindings && ts.isNamedImports(bindings))
113+
for (const element of bindings.elements) known.add(element.name.text);
114+
}
115+
const aliases = new Set<string>();
116+
let grew = true;
117+
while (grew) {
118+
grew = false;
119+
for (const [file, source] of Object.entries(readers)) {
120+
const visit = (node: ts.Node) => {
121+
if (ts.isTypeAliasDeclaration(node)) {
122+
const target = referenced(node.type);
123+
const name = qualifiedName(node);
124+
if (target !== null && known.has(target) && !aliases.has(name)) {
125+
aliases.add(name);
126+
known.add(node.name.text);
127+
grew = true;
128+
}
129+
}
130+
ts.forEachChild(node, visit);
131+
};
132+
visit(parse(file, source));
133+
}
134+
}
135+
return aliases;
136+
}
137+
138+
/**
139+
* The readers' aliases, written down: a name stays protected when its alias
140+
* is turned into a description, which a list read from the code alone would
141+
* lose along with the alias. The spec holds this list to the code both ways.
142+
*/
143+
const READER_ALIASES = [
144+
'ResultGroup',
145+
'Search.Entry',
146+
'Search.Facet',
147+
'Search.Icon.Entry',
148+
'Search.Icon.Result',
149+
'Search.PathwayOf',
150+
'Search.Result',
151+
'SearchResult',
152+
];
153+
154+
/** Every declaration in the readers that describes, again, an answer the contract has. */
155+
function secondDescriptions(readers: Record<string, string>, names: Set<string>): string[] {
156+
const found: string[] = [];
157+
// A contract name or a re-export's is protected wherever it is declared; an
158+
// alias's, by its qualified name.
159+
const isProtected = (node: ts.Node & { name: ts.Identifier }) =>
160+
names.has(node.name.text) || names.has(qualifiedName(node));
161+
for (const [file, source] of Object.entries(readers)) {
162+
const visit = (node: ts.Node) => {
163+
if (ts.isInterfaceDeclaration(node) && isProtected(node))
164+
found.push(`${file}: interface ${qualifiedName(node)}`);
165+
if (ts.isClassDeclaration(node) && node.name && isProtected({ ...node, name: node.name }))
166+
found.push(`${file}: class ${node.name.text}`);
167+
if (ts.isTypeAliasDeclaration(node) && isProtected(node) && describesShape(node.type))
168+
found.push(`${file}: type ${qualifiedName(node)}`);
169+
ts.forEachChild(node, visit);
170+
};
171+
visit(parse(file, source));
172+
}
173+
return found;
174+
}
175+
176+
const contract = Object.fromEntries(
177+
readdirSync(CONTRACT)
178+
.filter((file) => file.endsWith('.ts') && !file.endsWith('.spec.ts'))
179+
.map((file) => [file, readFileSync(path.join(CONTRACT, file), 'utf8')])
180+
);
181+
const readers = Object.assign({}, ...READERS.map(modules)) as Record<string, string>;
182+
const contractOwn = contractNames(contract);
183+
const aliases = localAliases(readers, contractOwn);
184+
const names = new Set([...contractOwn, ...readerAliases(readers), ...READER_ALIASES]);
185+
186+
describe('one description per answer', () => {
187+
it('knows the contract by its names and the names readers give it', () => {
188+
// The site reads the search entry's view as SearchEntry.
189+
expect([...names]).toEqual(
190+
expect.arrayContaining(['SearchEntryView', 'SearchEntry', 'FacetCount'])
191+
);
192+
});
193+
194+
it("lists the readers' aliases exactly, so one turned into a description is still caught", () => {
195+
expect([...aliases].sort()).toEqual([...READER_ALIASES].sort());
196+
});
197+
198+
it('finds no answer described again by the site or the pathway browser', () => {
199+
expect(secondDescriptions(readers, names)).toEqual([]);
200+
});
201+
202+
it('refuses an interface or an object type under a contract name, and allows an alias', () => {
203+
expect(
204+
secondDescriptions(
205+
{
206+
'a.ts': 'export interface SearchEntry { dbId: number }',
207+
'b.ts': 'type FacetCount = { name: string; count: number };',
208+
'c.ts': 'export namespace Search { export interface IconEntry { stId: string } }',
209+
'd.ts': "import type { FacetCount as F } from '../contract/search.ts';\ntype Facet = F;",
210+
'e.ts': 'type ResultGroup = Group<SearchEntryView>;',
211+
'f.ts': 'type Facet = Readonly<{ name: string }>;',
212+
'g.ts': 'type IconEntry = { [K in "a" | "b"]: string };',
213+
'h.ts': 'export namespace Search { export interface Entry { dbId: string } }',
214+
'i.ts': 'export class SearchEntry { dbId = 0; }',
215+
'j.ts': 'type FacetCount = ({ name: string })[];',
216+
'k.ts': 'type Facet = Record<string, FacetCount>;',
217+
},
218+
new Set(['SearchEntry', 'FacetCount', 'IconEntry', 'Facet', 'ResultGroup', 'Search.Entry'])
219+
)
220+
).toEqual([
221+
'a.ts: interface SearchEntry',
222+
'b.ts: type FacetCount',
223+
'c.ts: interface Search.IconEntry',
224+
'f.ts: type Facet',
225+
'g.ts: type IconEntry',
226+
'h.ts: interface Search.Entry',
227+
'i.ts: class SearchEntry',
228+
'j.ts: type FacetCount',
229+
]);
230+
});
231+
});

‎specs/016-api-contract/tasks.md‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -81,8 +81,11 @@ description: 'Tasks for spec 016: one API contract for the content service and t
8181

8282
**Goal**: the guarantee is shown, not assumed, for everything in the contract so far.
8383

84-
- [ ] T016 [US1] Add a spec that fails when a contract answer is also declared in `projects/website-angular/src/model` or `projects/pathway-browser` (the "no second description" check). Show it failing on a re-added local `interface SearchEntry`.
85-
- [ ] T017 [US1] Record in this file, with the commands, a rename of one field per moved family failing the writer and every reader ([quickstart.md](quickstart.md) §1).
84+
- [x] T016 [US1] Add a spec that fails when a contract answer is also declared in `projects/website-angular/src/model` or `projects/pathway-browser` (the "no second description" check). Show it failing on a re-added local `interface SearchEntry`. — #706. `projects/contract/one-description.spec.ts` takes the contract's exported names and the names the readers re-export them as (the site's `SearchEntry` is the contract's `SearchEntryView`), and refuses an interface or an object type under any of them in the site's or the pathway browser's code, anywhere (a namespace's too); an alias of the contract's type is a name, not a description. A re-added `interface SearchEntry` in the site's search model failed it, naming the file. From #705's review: the readers' own aliases are protected too (the pathway browser's `Search.Entry`, `Search.Result` and the rest, the site's `SearchResult`), by a written list the spec holds to the code both ways, so an alias turned into an interface is caught (shown on `Search.Entry`); and a shape written as `Readonly<{…}>`, a mapped type, in parentheses, as an array or as a class is a description too.
85+
- [x] T017 [US1] Record in this file, with the commands, a rename of one field per moved family failing the writer and every reader ([quickstart.md](quickstart.md) §1). — #706. The search family, by its three moves, each a field renamed in `projects/contract/search.ts` and the two programs checked (`npx tsc -p tools/tsconfig.json --noEmit`, `npx tsc -p tsconfig.app.json --noEmit`), then undone:
86+
- the facets (#684): `FacetCount.count` failed `tools/content-node/search-core.ts:80`, `search-endpoints.ts:96` and the site's `search/search.component.ts:339`;
87+
- the entry (#689): `exactType` failed the writer, `search-core.ts:257`, and the site's `search.component.ts:672` (the pathway browser reads it in its templates, which the site's production build checks);
88+
- the answers (#703): `GroupedResult.numberOfMatches` failed the writers, `search-endpoints.ts:287` and `:298`, and the site's `search.component.ts:326`, `:408`, `:415` and `:454`.
8689

8790
## Phase 6: User Story 4 - The description, generated (P2)
8891

0 commit comments

Comments
 (0)