Refactored binding wrapper

[kraenhansen]

What, How & Why?

As a prerequisite for the rebase of the WASM / browser binding, I had to rework the generated code that wraps the native module and expose it in a TypeScript friendlier way than free functions. Instead of having platform specific wrappers that implement platform specific code to load the native module I suggest exporting an injectNativeModule function which takes the native module (from somewhere) and wraps it into the binding namespace / object.

This has multiple benefits:

• We'll be able to maintain the platform specific code which load the native module as part of our src/platform files, which enables us to leverage TypeScript to achieve type-safety.
• We'll be able to call this injectNativeModule once the native module has loaded (possibly asynchronously as in the case of the WASM module).
• I've implemented this as a TypeScript source file, instead of the JS + d.ts separation that the current wrappers use. This had the potential to become more cumbersome to write, but it turned out fairly simple as the native module just has any type at the point of injection. This has the added benefit that we can get rid of a lot of the complexity in the ./binding directory as well as the babel transpile step require to get a CommonJS variant of the wrappers.

☑️ ToDos

• 📝 Changelog entry
• 📝 Compatibility label is updated or copied from previous entry
• 📝 Update COMPATIBILITY.md
• 🚦 Tests
• 📦 Updated internal package version in consuming package.jsons (if updating internal packages)
• 📱 Check the React Native/other sample apps work if necessary
• 💥 Breaking label has been applied or is not necessary
• 🔔 Mention @realm/devdocs if documentation changes are needed

[kraenhansen]

I rebased this on main as #6814 was merged.

[kraenhansen]

The removal of the platform specific wrappers and the addition of this file are the biggest part of the changes proposed here. Originally we had the thought that keeping these separate could allow for platform specific optimizations but in reality we haven't relied on that and the separation gives rise to extra unneeded complexity.

[elle-j]

Nice refactor 🙂 LGTM after addressing the comments 👌

[elle-j]

Suggested change

	import { TemplateContext } from "@realm/bindgen/context";
	////////////////////////////////////////////////////////////////////////////
	//
	// Copyright 2024 Realm Inc.
	//
	// Licensed under the Apache License, Version 2.0 (the "License");
	// you may not use this file except in compliance with the License.
	// You may obtain a copy of the License at
	//
	// http://www.apache.org/licenses/LICENSE-2.0
	//
	// Unless required by applicable law or agreed to in writing, software
	// distributed under the License is distributed on an "AS IS" BASIS,
	// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
	// See the License for the specific language governing permissions and
	// limitations under the License.
	//
	////////////////////////////////////////////////////////////////////////////
	import { TemplateContext } from "@realm/bindgen/context";

[kraenhansen]

This is missing because the eslint config is different for these files: https://github.com/realm/realm-js/blob/nh/wasm/emscripten_target/packages/realm/bindgen/.eslintrc.json

We could probably delete that .eslintrc.json and commit the --fix but I'll defer that to another PR.

[elle-j]

Was there a reason for including undefined here?

Suggested change

	const PRIMITIVES_MAPPING: Record<string, string | undefined> = {
	const PRIMITIVES_MAPPING: Record<string, string> = {

[kraenhansen]

Because not all possible string properties result in a string upon lookup.

If omitted PRIMITIVES_MAPPING["invalid"] would have the type string when in fact it'll be undefined. I often forget this myself (and should probably add it to TEMPLATE_MAPPING as well 🤔)

[elle-j]

Nice that generateXDeclaration logic has been pulled out 👍

[elle-j]

Nit: Alternatively, this could be refactored into some assertSupportedTypes() (or the like) function (with the rawSpec param), and let generate() only focus on generating output.

[kraenhansen]

I agree this could be simplified, but I think I'll let it pass for now as it's a copy-paste from the former typescript.ts template:

realm-js/packages/realm/bindgen/src/templates/typescript.ts

Lines 135 to 150 in ff3d889

	// Check the support for primitives used
	for (const primitive of rawSpec.primitives) {
	if (primitive === "Mixed") continue;
	assert(
	Object.keys(PRIMITIVES_MAPPING).includes(primitive),
	`Spec declares an unsupported primitive: "${primitive}"`,
	);
	}
	// Check the support for template instances used
	for (const template of Object.keys(rawSpec.templates)) {
	assert(
	Object.keys(TEMPLATE_MAPPING).includes(template),
	`Spec declares an unsupported template instance: "${template}"`,
	);
	}

[elle-j]

Nice simplification here

[elle-j]

Interesting solution for the RN issue 🙂

[elle-j]

Is this also a polyfill? If so, perhaps we could declare and define it PolyfillWeakRef similar to the bigint polyfill.