Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Handle >2 union members in links #669

Merged
merged 6 commits into from
Jul 11, 2023
Merged

Handle >2 union members in links #669

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
6750956
Handle >2 union members in links
scotttrinh Jun 26, 2023
3d8318a
Add 2.x back in but continue on error
scotttrinh Jun 26, 2023
cdc9662
wip: Attempt to find the macOS test failure
scotttrinh Jun 26, 2023
30a158c
Revert allowing 2.x to fail
scotttrinh Jun 28, 2023
42b4595
Run tests on all pull requests
scotttrinh Jun 28, 2023
2c91512
Commit the migration
scotttrinh Jun 28, 2023
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
2 changes: 0 additions & 2 deletions .github/workflows/tests.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,6 @@ on:
- master
- ci
pull_request:
branches:
- master

jobs:
test:
Expand Down
6 changes: 5 additions & 1 deletion integration-tests/lts/dbschema/default.esdl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,10 @@ module default {

type Simple extending HasName, HasAge {}

type W {
property a -> str;
property d -> float64;
}
type X {
property a -> str;
property b -> int32;
Expand All @@ -137,7 +141,7 @@ module default {
property c -> bool;
}
type Z {
link xy -> X | Y;
link xy -> W | X | Y;
}

# Unicode handling
Expand Down
17 changes: 17 additions & 0 deletions integration-tests/lts/dbschema/migrations/00021.edgeql
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
CREATE MIGRATION m1bffqrfcj7ols7s3v27kgbxhtsetpvwvqpxrogvhsq2crwwlnbbya
ONTO m1sxhoqfjqn7vtpmatzanmwydtxndf3jlf33npkblmya42fx3bcdoa
{
CREATE TYPE default::W {
CREATE PROPERTY a -> std::str;
CREATE PROPERTY d -> std::float64;
};
ALTER TYPE default::Z {
ALTER LINK xy {
SET TYPE ((default::Y | default::X) | default::W) USING (SELECT
default::X
LIMIT
1
);
};
};
};
4 changes: 2 additions & 2 deletions integration-tests/lts/interfaces.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import * as tc from "conditional-type-checks";

import type { Movie, X, Y, Z } from "./dbschema/interfaces";
import type { Movie, W, X, Y, Z } from "./dbschema/interfaces";

export type Genre =
| "Horror"
Expand Down Expand Up @@ -32,7 +32,7 @@ export interface test_Profile extends BaseObject {
c?: string | null;
}
interface test_Z extends BaseObject {
xy?: X | Y | null;
xy?: W | X | Y | null;
}

describe("interfaces", () => {
Expand Down
5 changes: 4 additions & 1 deletion integration-tests/lts/objectTypes.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,10 @@ describe("object types", () => {
$Z.__pointers__.xy.target.__pointers__.a.target.__name__,
"std::str"
);
assert.equal($Z.__pointers__.xy.target.__name__, "default::X | default::Y");
assert.equal(
$Z.__pointers__.xy.target.__name__,
"default::X | default::Y | default::W"
);
});

const link = $AnnotationSubject.__pointers__.annotations;
Expand Down
2 changes: 1 addition & 1 deletion integration-tests/lts/params.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -274,7 +274,7 @@ SELECT (SELECT __param__test)`
tuple: args,
});

assert.deepEqual(Object.values(complexResult.tuple), Object.values(args));
assert.deepEqual(complexResult.tuple, args);
});

test("v2 param types", async () => {
Expand Down
186 changes: 2 additions & 184 deletions packages/generate/src/edgeql-js/generateObjectTypes.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { CodeFragment, dts, r, t, ts } from "../builders";
import { type CodeFragment, dts, r, t, ts } from "../builders";
import type { GeneratorParams } from "../genutil";
import type { $ } from "../genutil";
import {
Expand Down Expand Up @@ -158,93 +158,15 @@ export const getStringRepresentation: (
export const generateObjectTypes = (params: GeneratorParams) => {
const { dir, types } = params;

// const plainTypesCode = dir.getPath("types");
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah, I'm just killing all this commented out code: it's already drifting pretty far, so I don't think it makes much sense to keep it.

// plainTypesCode.addImportStar("edgedb", "edgedb", {
// typeOnly: true
// });
// const plainTypeModules = new Map<
// string,
// {internalName: string; buf: CodeBuffer; types: Map<string, string>}
// >();

// const getPlainTypeModule = (
// typeName: string
// ): {
// tMod: string;
// tName: string;
// module: {
// internalName: string;
// buf: CodeBuffer;
// types: Map<string, string>;
// };
// } => {
// const {mod: tMod, name: tName} = splitName(typeName);
// if (!plainTypeModules.has(tMod)) {
// plainTypeModules.set(tMod, {
// internalName: makePlainIdent(tMod),
// buf: new CodeBuffer(),
// types: new Map()
// });
// }
// return {tMod, tName, module: plainTypeModules.get(tMod)!};
// };

// const _getTypeName =
// (mod: string) =>
// (typeName: string, withModule: boolean = false): string => {
// const {tMod, tName, module} = getPlainTypeModule(typeName);
// return (
// ((mod !== tMod || withModule) && tMod !== "default"
// ? `${module.internalName}.`
// : "") + `${makePlainIdent(tName)}`
// );
// };

for (const type of types.values()) {
if (type.kind !== "object") {
// if (type.kind === "scalar" && type.enum_values?.length) {
// // generate plain enum type
// const {mod: enumMod, name: enumName} = splitName(type.name);
// const getEnumTypeName = _getTypeName(enumMod);

// const {module} = getPlainTypeModule(type.name);
// module.types.set(enumName, getEnumTypeName(type.name, true));
// module.buf.writeln(
// [t`export enum ${getEnumTypeName(type.name)} {`],
// ...type.enum_values.map(val => [
// t` ${makePlainIdent(val)} = ${quote(val)},`
// ]),
// [t`}`]
// );

// if (enumMod === "default") {
// module.buf.writeln(
// [js`const ${getEnumTypeName(type.name)} = {`],
// ...type.enum_values.map(val => [
// js` ${makePlainIdent(val)}: ${quote(val)},`
// ]),
// [js`}`]
// );
// plainTypesCode.addExport(getEnumTypeName(type.name), {
// modes: ["js"]
// });
// } else {
// module.buf.writeln(
// [js`"${getEnumTypeName(type.name)}": {`],
// ...type.enum_values.map(val => [
// js` ${makePlainIdent(val)}: ${quote(val)},`
// ]),
// [js`},`]
// );
// }
// }
continue;
}

const isUnionType = Boolean(type.union_of?.length);
const isIntersectionType = Boolean(type.intersection_of?.length);

if (isIntersectionType) {
if (isIntersectionType || isUnionType) {
Copy link
Collaborator Author

@scotttrinh scotttrinh Jun 26, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Here's the actual fix: we just skip generating any type if it's actually a union instead of a separate type.

continue;
}

Expand All @@ -256,65 +178,6 @@ export const generateObjectTypes = (params: GeneratorParams) => {

const ref = getRef(type.name);

/////////
// generate plain type
/////////

// const getTypeName = _getTypeName(mod);

// const getTSType = (pointer: $.introspect.Pointer): string => {
// const targetType = types.get(pointer.target_id);
// if (pointer.kind === "link") {
// return getTypeName(targetType.name);
// } else {
// return toTSScalarType(
// targetType as $.introspect.PrimitiveType,
// types,
// {
// getEnumRef: enumType => getTypeName(enumType.name),
// edgedbDatatypePrefix: ""
// }
// ).join("");
// }
// };

// const {module: plainTypeModule} = getPlainTypeModule(type.name);

// if (!isUnionType) {
// plainTypeModule.types.set(name, getTypeName(type.name, true));
// }
// plainTypeModule.buf.writeln([
// t`${
// !isUnionType ? "export " : ""
// }interface ${getTypeName(type.name)}${
// type.bases.length
// ? ` extends ${type.bases
// .map(({id}) => {
// const baseType = types.get(id);
// return getTypeName(baseType.name);
// })
// .join(", ")}`
// : ""
// } ${
// type.pointers.length
// ? `{\n${type.pointers
// .map(pointer => {
// const isOptional =
// pointer.real_cardinality === Cardinality.AtMostOne;
// return ` ${quote(pointer.name)}${
// isOptional ? "?" : ""
// }: ${getTSType(pointer)}${
// pointer.card === Cardinality.Many ||
// pointer.card === Cardinality.AtLeastOne
// ? "[]"
// : ""
// }${isOptional ? " | null" : ""};`;
// })
// .join("\n")}\n}`
// : "{}"
// }\n`,
// ]);

/////////
// generate interface
/////////
Expand Down Expand Up @@ -357,19 +220,6 @@ export const generateObjectTypes = (params: GeneratorParams) => {
};
};

// unique
// const BaseObject = params.typesByName["std::BaseObject"];
// const uniqueStubs = [...new Set(type.backlinks.map((bl) => bl.stub))];
// const stubLines = uniqueStubs.map((stub): $.introspect.Pointer => {
// return {
// card: Cardinality.Many,
// kind: "link",
// name: `<${stub}`,
// target_id: BaseObject.id,
// is_exclusive: false,
// pointers: null,
// };
// });
const lines = [
...type.pointers,
...type.backlinks,
Expand Down Expand Up @@ -456,7 +306,6 @@ export const generateObjectTypes = (params: GeneratorParams) => {
}
});

// const ref = getRef(type.name);
for (const ex of type.exclusives) {
body.writeln([
t` {`,
Expand All @@ -468,7 +317,6 @@ export const generateObjectTypes = (params: GeneratorParams) => {
}),
t`},`,
]);
// body.writeln([t`\n {${lines.join(", ")}}`]);
}

body.writeln([t`]>;`]);
Expand All @@ -480,11 +328,6 @@ export const generateObjectTypes = (params: GeneratorParams) => {
/////////
// generate runtime type
/////////
if (isUnionType) {
// union types don't need runtime type
continue;
}

const literal = getRef(type.name, { prefix: "" });

body.writeln([
Expand All @@ -496,7 +339,6 @@ export const generateObjectTypes = (params: GeneratorParams) => {
r`(_.spec, ${quote(type.id)}, _.syntax.literal);`,
]);
body.addExport(ref);
// body.addExport(ref, `$${name}`); // dollar

const typeCard = singletonObjectTypes.has(type.name) ? "One" : "Many";

Expand All @@ -513,28 +355,4 @@ export const generateObjectTypes = (params: GeneratorParams) => {
body.addExport(literal);
body.addToDefaultExport(literal, name);
}

// plain types export
// const plainTypesExportBuf = new CodeBuffer();
// for (const [moduleName, module] of plainTypeModules) {
// if (moduleName === "default") {
// plainTypesCode.writeBuf(module.buf);
// } else {
// plainTypesCode.writeln([t`export namespace ${module.internalName} {`]);
// plainTypesCode.writeln([js`const ${module.internalName} = {`]);
// plainTypesCode.indented(() => plainTypesCode.writeBuf(module.buf));
// plainTypesCode.writeln([t`}`]);
// plainTypesCode.writeln([js`}`]);
// plainTypesCode.addExport(module.internalName, {modes: ["js"]});
// }

// plainTypesExportBuf.writeln([
// t` ${quote(moduleName)}: {\n${[...module.types.entries()]
// .map(([name, typeName]) => ` ${quote(name)}: ${typeName};`)
// .join("\n")}\n };`
// ]);
// }
// plainTypesCode.writeln([t`export interface types {`]);
// plainTypesCode.writeBuf(plainTypesExportBuf);
// plainTypesCode.writeln([t`}`]);
};