Skip to content

Commit

Permalink
Migrating from util::Optional to std::optional since Realm Core i…
Browse files Browse the repository at this point in the history
…s moving that way (#6449)

* Migrating from `tul::Optional` to `std::optional` since Realm Core is
moving that way
* Update core
* Update CHANGELOG.md
* Make linter great again
  • Loading branch information
kneth authored Feb 9, 2024
1 parent d056226 commit 02e0f02
Show file tree
Hide file tree
Showing 12 changed files with 428 additions and 157 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
<!-- * Using Realm Core vX.Y.Z -->
<!-- * Upgraded Realm Core from vX.Y.Z to vA.B.C -->
* Fix Cocoapods to version 1.14.3 on Github Actions.
* Migrated bingen from `util::Optional` to `std::optional`.

## 12.6.0 (2024-01-29)

Expand Down
528 changes: 386 additions & 142 deletions package-lock.json

Large diffs are not rendered by default.

14 changes: 12 additions & 2 deletions packages/realm/bindgen/src/js-passes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,12 @@ function addSharedPtrMethods(spec: BoundSpec) {
}

class CustomProperty extends Property {
constructor(on: Class, public readonly name: string, type: Type, public call: MethodCallSig) {
constructor(
on: Class,
public readonly name: string,
type: Type,
public call: MethodCallSig,
) {
assert(name.startsWith("$"));
super(on, "DOLLAR_" + name.slice(1), type);
}
Expand All @@ -82,7 +87,12 @@ class CustomProperty extends Property {
}

class CustomInstanceMethod extends InstanceMethod {
constructor(on: Class, public name: string, sig: Func, public call: MethodCallSig) {
constructor(
on: Class,
public name: string,
sig: Func,
public call: MethodCallSig,
) {
assert(name.startsWith("$"));
const unique_name = "DOLLAR_" + name.slice(1);
super(on, name, unique_name, unique_name, sig);
Expand Down
12 changes: 8 additions & 4 deletions packages/realm/bindgen/src/templates/jsi.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,11 @@ function tryWrap(body: string) {
}

class CppJsiFunc extends CppFunc {
constructor(private addon: JsiAddon, name: string, props?: CppFuncProps) {
constructor(
private addon: JsiAddon,
name: string,
props?: CppFuncProps,
) {
super(name, "jsi::Value", jsi_callback_args, props);
}

Expand Down Expand Up @@ -430,7 +434,7 @@ function convertToJsi(addon: JsiAddon, type: Type, expr: string): string {
return c(new Pointer(inner), expr);
case "Nullable":
return `[&] (auto&& val) { return !val ? jsi::Value::null() : ${c(inner, "FWD(val)")}; }(${expr})`;
case "util::Optional":
case "std::optional":
return `[&] (auto&& opt) { return !opt ? jsi::Value::undefined() : ${c(inner, "*FWD(opt)")}; }(${expr})`;
case "std::vector":
// TODO try different ways to create the array to see what is fastest.
Expand Down Expand Up @@ -558,7 +562,7 @@ function convertFromJsi(addon: JsiAddon, type: Type, expr: string): string {
return `[&] (auto&& val) {
return val.isNull() ? ${inner.toCpp()}() : ${c(inner, "FWD(val)")};
}(${expr})`;
case "util::Optional":
case "std::optional":
return `[&] (auto&& val) {
return val.isUndefined() ? ${type.toCpp()}() : ${c(inner, "FWD(val)")};
}(${expr})`;
Expand Down Expand Up @@ -955,7 +959,7 @@ class JsiCppDecls extends CppDecls {
.getProperty(_env, "name").asString(_env).utf8(_env);
throw jsi::JSError(_env, "Unable to convert an object with ctor '" + ctorName + "' to a Mixed");
}
// NOTE: must not treat undefined as null here, because that makes Optional<Mixed> ambiguous.
// NOTE: must not treat undefined as null here, because that makes optional<Mixed> ambiguous.
throw jsi::JSError(_env, "Can't convert " + val.toString(_env).utf8(_env) + " to Mixed");
`,
}),
Expand Down
1 change: 1 addition & 0 deletions packages/realm/bindgen/src/templates/node-wrapper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import { TemplateContext } from "@realm/bindgen/context";
import { doJsPasses } from "../js-passes";
import { eslint } from "../eslint-formatter";

// eslint-disable-next-line @typescript-eslint/no-unused-vars
export function generate({ rawSpec, spec: boundSpec, file }: TemplateContext): void {
const spec = doJsPasses(boundSpec);
const reactLines = [];
Expand Down
12 changes: 8 additions & 4 deletions packages/realm/bindgen/src/templates/node.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,11 @@ function tryWrap(body: string) {
}

class CppNodeFunc extends CppFunc {
constructor(private addon: NodeAddon, name: string, props?: CppFuncProps) {
constructor(
private addon: NodeAddon,
name: string,
props?: CppFuncProps,
) {
super(name, "Napi::Value", [node_callback_info], props);
}

Expand Down Expand Up @@ -367,7 +371,7 @@ function convertToNode(addon: NodeAddon, type: Type, expr: string): string {
return c(new Pointer(inner), expr);
case "Nullable":
return `[&] (auto&& val) { return !val ? ${env}.Null() : ${c(inner, "FWD(val)")}; }(${expr})`;
case "util::Optional":
case "std::optional":
return `[&] (auto&& opt) { return !opt ? ${env}.Undefined() : ${c(inner, "*FWD(opt)")}; }(${expr})`;
case "std::vector":
// TODO try different ways to create the array to see what is fastest.
Expand Down Expand Up @@ -480,7 +484,7 @@ function convertFromNode(addon: NodeAddon, type: Type, expr: string): string {
return `std::make_shared<${inner.toCpp()}>(${c(inner, expr)})`;
case "Nullable":
return `[&] (Napi::Value val) { return val.IsNull() ? ${inner.toCpp()}() : ${c(inner, "val")}; }(${expr})`;
case "util::Optional":
case "std::optional":
return `[&] (Napi::Value val) {
return val.IsUndefined() ? ${type.toCpp()}() : ${c(inner, "val")};
}(${expr})`;
Expand Down Expand Up @@ -889,7 +893,7 @@ class NodeCppDecls extends CppDecls {
const auto ctorName = obj.Get("constructor").As<Napi::Object>().Get("name").As<Napi::String>().Utf8Value();
throw Napi::TypeError::New(${env}, "Unable to convert an object with ctor '" + ctorName + "' to a Mixed");
}
// NOTE: must not treat undefined as null here, because that makes Optional<Mixed> ambiguous.
// NOTE: must not treat undefined as null here, because that makes optional<Mixed> ambiguous.
${["undefined", "symbol", "function", "external"]
.map((t) => `case napi_${t}: throw Napi::TypeError::New(${env}, "Can't convert ${t} to Mixed");`)
.join("\n")}
Expand Down
6 changes: 3 additions & 3 deletions packages/realm/bindgen/src/templates/typescript.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ const PRIMITIVES_MAPPING: Record<string, string> = {
// Be Careful! These need to apply to the *whole* type, so arg[] would be problematic if arg is A|B.
const TEMPLATE_MAPPING: Record<string, (...args: string[]) => string> = {
"std::vector": (arg) => `Array<${arg}>`,
"util::Optional": (arg) => `undefined | ${arg}`,
"std::optional": (arg) => `undefined | ${arg}`,
Nullable: (t) => `null | ${t}`,
"std::shared_ptr": (arg) => arg,
"std::pair": (a, b) => `[${a}, ${b}]`,
Expand Down Expand Up @@ -230,8 +230,8 @@ export function generate({ rawSpec, spec: boundSpec, file }: TemplateContext): v
// TODO consider making the Arg version just alias the Ret version if the bodies are the same.
out(`export type ${rec.jsName}${suffix(kind)} = {`);
for (const field of fields) {
// For Optional<T> fields, the field will always be there in Ret mode, but it may be undefined.
// This is handled by Optional<T> becoming `undefined | T`.
// For optional<T> fields, the field will always be there in Ret mode, but it may be undefined.
// This is handled by optional<T> becoming `undefined | T`.
const optField = !field.required && kind === Kind.Arg;
const hasInterestingDefault = ![undefined, "", "{}", "[]"].includes(field.defaultVal);

Expand Down
5 changes: 5 additions & 0 deletions packages/realm/src/Realm.ts
Original file line number Diff line number Diff line change
Expand Up @@ -906,6 +906,7 @@ export class Realm {
* Deletes the provided Realm object, or each one inside the provided collection.
* @param subject - The Realm object to delete, or a collection containing multiple Realm objects to delete.
*/
// eslint-disable-next-line @typescript-eslint/no-explicit-any
delete(subject: AnyRealmObject | AnyRealmObject[] | AnyList | AnyResults | any): void {
assert.inTransaction(this, "Can only delete objects within a transaction.");
assert.object(subject, "subject");
Expand Down Expand Up @@ -1597,6 +1598,7 @@ export declare namespace Realm {
export type CountOptions = CountOptionsType;
export type DeleteEvent<T extends Document> = DeleteEventType<T>;
export type DeleteResult = DeleteResultType;
// eslint-disable-next-line @typescript-eslint/no-explicit-any
export type Document<IdType = any> = DocumentType<IdType>;
export type DocumentKey<IdType> = DocumentKeyType<IdType>;
export type DocumentNamespace = DocumentNamespaceType;
Expand Down Expand Up @@ -1639,6 +1641,7 @@ export declare namespace Realm {
export type Set<T> = Realm.Set<T>;
export type Dictionary<T> = Realm.Dictionary<T>;
export type Mixed = unknown;
// eslint-disable-next-line @typescript-eslint/no-unused-vars
export type LinkingObjects<ObjectTypeT, LinkingPropertyName> = Realm.Results<ObjectTypeT>;
}
}
Expand Down Expand Up @@ -1815,6 +1818,7 @@ declare global {
export type CountOptions = CountOptionsType;
export type DeleteEvent<T extends Document> = DeleteEventType<T>;
export type DeleteResult = DeleteResultType;
// eslint-disable-next-line @typescript-eslint/no-explicit-any
export type Document<IdType = any> = DocumentType<IdType>;
export type DocumentKey<IdType> = DocumentKeyType<IdType>;
export type DocumentNamespace = DocumentNamespaceType;
Expand Down Expand Up @@ -1857,6 +1861,7 @@ declare global {
export type Set<T> = Realm.Set<T>;
export type Dictionary<T> = Realm.Dictionary<T>;
export type Mixed = unknown;
// eslint-disable-next-line @typescript-eslint/no-unused-vars
export type LinkingObjects<ObjectTypeT, LinkingPropertyName> = Realm.Results<ObjectTypeT>;
}
}
Expand Down
1 change: 1 addition & 0 deletions packages/realm/src/Unmanaged.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
//
////////////////////////////////////////////////////////////////////////////

// eslint-disable-next-line @typescript-eslint/no-unused-vars
import { Collection, Dictionary, List, RealmObject } from "./internal";
import { AnyRealmObject } from "./Object";

Expand Down
2 changes: 1 addition & 1 deletion packages/realm/src/app-services/App.ts
Original file line number Diff line number Diff line change
Expand Up @@ -267,7 +267,7 @@ export class App<
constructor(configOrId: AppConfiguration | string) {
const config: AppConfiguration = typeof configOrId === "string" ? { id: configOrId } : configOrId;
assert.object(config, "config");
const { id, baseUrl, app, timeout, multiplexSessions = true, baseFilePath, metadata, fetch } = config;
const { id, baseUrl, timeout, multiplexSessions = true, baseFilePath, metadata, fetch } = config;
assert.string(id, "id");
if (timeout !== undefined) {
assert.number(timeout, "timeout");
Expand Down
1 change: 1 addition & 0 deletions packages/realm/src/tests/schema-transform.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ describe("schema transform", () => {
linkOriginPropertyName: "",
isPrimary: false,
isIndexed: false,
// eslint-disable-next-line @typescript-eslint/no-explicit-any
columnKey: { value: 0n } as any,
...bindingSchema,
});
Expand Down

0 comments on commit 02e0f02

Please sign in to comment.