CommonJS bundle (without Rollup) #5853

kraenhansen · 2023-05-26T13:51:00Z

What, How & Why?

This fixed #5857 by removing rollup from the SDK package in favour of a TypeScript projects based setup which emits only CommonJS code.

This is aligned with recent discussions on the team to ensure v12.0.0 exports exactly as v11 did, to keep friction from migrating at an absolute minimum.

I ran into several problems of which some I managed to find workarounds for and others I didn't:

The Typescript compiler would emit .mjs files even when module was set to commonjs which would break the package upon import. I filed an issue for this, created a PR and found a temporary workaround.
Although the Jest issue is closed, I found problems from Jest lacking support of export conditions out of the box and had to implement a custom resolver. I later transitioned into exporting via a separate package in an attempt to work around this.
I was planning to rely on bundleDependencies to pull @realm/network-transport (and a new @realm/binding package) into the SDK's .tar.gz before uploading it to NPM. But this is not supported when using NPM workspaces (yet) :meow_sad: [BUG] Workspaces - bundledDependencies missing in tarball after npm pack npm/cli#3466

kraenhansen · 2023-05-26T13:55:45Z

Woups - looks like I need to find a solution for re-enabling test coverage 🤔

elle-j · 2023-05-30T08:58:20Z

packages/realm/package.json

+      "react-native": "./generated/native-react-native.mjs"
+    },
+    "./binding/core": "./dist/generated/core.js",


Can the core part be moved into the other ".binding" block?

Suggested change

"react-native": "./generated/native-react-native.mjs"

},

"./binding/core": "./dist/generated/core.js",

"react-native": "./generated/native-react-native.mjs",

"core": "./dist/generated/core.js"

},

RedBeard0531 · 2023-05-30T10:58:06Z

.gitignore

 /packages/realm/generated/
+/packages/realm/src/generated/


It seems silly to have two separate generated directories here. Can they be combined?

I needed to move core.ts into src when adopting TypeScript projects, because of:

error TS6059: File '/Users/kraen.hansen/Projects/realm-js/packages/realm/generated/core.ts' is not under 'rootDir' '/Users/kraen.hansen/Projects/realm-js/packages/realm/src'. 'rootDir' is expected to contain all source files.
The file is in the program because:
Matched by include pattern '../generated/core.ts' in '/Users/kraen.hansen/Projects/realm-js/packages/realm/src/tsconfig.json'

And I don't really consider the wrapper .mjs files and realm.node file as "src" as it's not TS and won't be moved to "dist" by tsc. Adding to that, I don't know yet if we actually want to add the src files to the archive that we upload to NPM (lots of packages don't - most likely to keep their size and complexity low).

Hmm. I'm not sure we have a clear rule for what src/ is. Is it where we put the "source code" for the realm package, or is it only the input for tsc? I had assumed the former, but it seems like you intend for it to be the later. I guess I don't feel too strongly either way, but we should be clear what our rule is.

RedBeard0531 · 2023-05-30T11:03:27Z

packages/realm/bindgen/CMakeLists.txt

@@ -125,7 +125,7 @@ file(GLOB_RECURSE SDK_TS_FILES
 list(FILTER SDK_TS_FILES EXCLUDE REGEX "templates/[^/]*\.ts$")

 set(JS_SPEC_FILE ${SDK_DIR}/bindgen/js_spec.yml)
-set(TYPESCRIPT_OUTPUT_DIR ${SDK_DIR}/generated/ts)
+set(TYPESCRIPT_OUTPUT_DIR ${SDK_DIR}/generated)


Shouldn't this be src/generated?

RedBeard0531 · 2023-05-30T11:04:43Z

packages/realm/src/Realm.ts

@@ -1433,6 +1437,7 @@ export declare namespace Realm {
    CollectionChangeSet,
    /** @deprecated Please use named imports */
    CollectionType as Collection,
+    CompensatingWriteErrorType,


Suggested change

CompensatingWriteErrorType,

CompensatingWriteErrorType as CompensatingWriteError,

RedBeard0531 · 2023-05-30T11:08:03Z

packages/realm/src/platform/node/tsconfig.json

    "types": ["node"],
-    "noResolve": false,
-    "incremental": true,


Do we want to remove incremental here? It does seem to speed up incremental building.

As I understand it (and as I can see tsbuildinfo files on disk), it's default for a composite config.
It's not explicit from the documentation though: https://www.typescriptlang.org/docs/handbook/project-references.html

RedBeard0531 · 2023-05-30T11:41:15Z

packages/realm/src/cjs.ts

What is with mixing the index.cjs.ts and cjs.ts naming conventions? Can we pick one. Also do we need both index.cjs.ts and index.ts? or could they be combined?

do we need both index.cjs.ts and index.ts? or could they be combined?

I wanted the ensure the general code-base was ESM and the cjs.ts was only to export an entrypoint for the types. I'll align the naming though 👍

(FYI in markdown you need a blank line after the last quoted line in order to end a quote and start your reply)

I wanted the ensure the general code-base was ESM and the cjs.ts was only to export an entrypoint for the types. I'll align the naming though

We still could get rid of index.ts and move its minimal contents to index.cjs.ts. It just seems weird to have both with the same root name. If you really want to keep as much as possible out of cjs files, it may be best to rename index.ts to reexport.ts or umbrella.ts.

integration-tests/tests/package.json

RedBeard0531 · 2023-06-06T11:08:49Z

packages/realm/bindgen/CMakeLists.txt

+    ${GENERATED_OUTPUT_DIR}/native.d.mts
    ${TYPESCRIPT_OUTPUT_DIR}/core.ts


I don't see how that can be right. These are build by the same command and it is only told about the path to TYPESCRIPT_OUTPUT_DIR.

True - I was missing a replace here 👍

RedBeard0531 · 2023-06-06T11:10:41Z

packages/realm/bindgen/CMakeLists.txt

    OUTPUTS ${GENERATED_JS_FILES}
-    OUTDIR ${TYPESCRIPT_OUTPUT_DIR}
+    OUTDIR ${GENERATED_OUTPUT_DIR}


I think you meant to change lines 144/145 instead. Right now there are out of sync.

… SDK root

kraenhansen · 2023-06-12T10:18:25Z

Drafting this as it's been superseded by #5882 .. I might close when I accept my defeat and feel better about the time I've already sunk into this.

takameyer · 2023-06-15T08:09:21Z

I'm guessing we are going to close this, or?

kraenhansen · 2023-06-15T08:10:56Z

@takameyer see my previous comment 🙂

kraenhansen · 2023-06-16T09:23:38Z

Closing this as I've created #5894 to eventually do this migration.
(I'll let the branch stick around for a bit longer).

kraenhansen requested review from RedBeard0531 and takameyer May 26, 2023 13:51

kraenhansen self-assigned this May 26, 2023

cla-bot bot added the cla: yes label May 26, 2023

kraenhansen added T-Internal and removed cla: yes labels May 26, 2023

kraenhansen mentioned this pull request May 26, 2023

Moved injecting code for Realm global to a separate file #5852

Closed

kraenhansen force-pushed the kh/cjs-bundle branch from 5791bad to 0b3f9ea Compare May 26, 2023 13:57

cla-bot bot added the cla: yes label May 26, 2023

elle-j approved these changes May 30, 2023

View reviewed changes

RedBeard0531 reviewed May 30, 2023

View reviewed changes

takameyer reviewed May 31, 2023

View reviewed changes

integration-tests/tests/package.json Outdated Show resolved Hide resolved

takameyer approved these changes May 31, 2023

View reviewed changes

RedBeard0531 reviewed Jun 6, 2023

View reviewed changes

kraenhansen added 14 commits June 7, 2023 14:42

Using tsc to build instead of rollup

7bd4c0e

Using default import of Realm in tests

f35276b

Adding missing NS exports

586b1f6

Renamed "realm:bundle" to "realm:build:ts"

367ca90

Fixing wireit deps for "build:ts"

f467de0

Using inline source maps

72ff354

Updated nyc config to reflect us using tsc

d2195f2

Moved realm:build:ts to build-dependencies

4c637c8

Removing code coverage

2325ab5

Using source maps files

18c3b1e

Introducing "GENERATED_OUTPUT_DIR" to CMakeLists.txt

ac046b9

Generating public types with internals stripped

7550498

Fixing CMakeLists.txt

ef7ac59

Invoking cmake from package.json scripts to generate TS and wrappers

6a8ad90

kraenhansen force-pushed the kh/cjs-bundle branch from 4ca8407 to d83f22a Compare June 7, 2023 13:25

kraenhansen added 8 commits June 7, 2023 16:31

Build ts pre-testing to allow "realm/binding/core"

cffdcba

Using AnyUser in SyncConfiguration

a050c03

Adjusted the location of the prebuilt

5e7e9a7

Renamed cjs.ts to index.cjs.ts

37164a4

Generating all .js and .ts into src/generated and realm.node into the…

511bd95

… SDK root

Working around microsoft/TypeScript#54573

a7a2158

Fixed @realm/react tests

7280d8c

Removed nyc

923206a

kraenhansen requested a review from takameyer June 9, 2023 17:26

kraenhansen added 9 commits June 9, 2023 19:39

Merge branch 'main' into kh/cjs-bundle

88c9e4e

Update package.json

c5a6fb6

Exposing PropertyTypeName to fix @realm/babel-plugin tests

6e72b4e

Exposing NumericLogLevel and WaitForSync on Realm

4e3c150

Exposing SubscriptionSetState

6cfd781

package-lock updated

d175251

Adding polyfills for conditional exports

ced8b15

Moved binding to @realm/binding

2100075

Adding the task to configure cmake

526a8af

kraenhansen mentioned this pull request Jun 12, 2023

Only export a CJS bundle (using rollup) #5882

Merged

4 tasks

kraenhansen changed the title ~~CommonJS bundle~~ CommonJS bundle (without Rollup) Jun 12, 2023

kraenhansen marked this pull request as draft June 12, 2023 10:17

kraenhansen mentioned this pull request Jun 16, 2023

Migrate SDK package to TypeScript "Project References" #5894

Closed

kraenhansen closed this Jun 16, 2023

kraenhansen mentioned this pull request Feb 16, 2024

Replace "rollup" bundling with TypeScript project references #6492

Merged

7 tasks

github-actions bot locked as resolved and limited conversation to collaborators Mar 15, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

CommonJS bundle (without Rollup) #5853

CommonJS bundle (without Rollup) #5853

kraenhansen commented May 26, 2023 •

edited

Loading

kraenhansen commented May 26, 2023

elle-j May 30, 2023

RedBeard0531 May 30, 2023

kraenhansen May 31, 2023 •

edited

Loading

RedBeard0531 Jun 2, 2023

RedBeard0531 May 30, 2023

RedBeard0531 May 30, 2023

RedBeard0531 May 30, 2023

kraenhansen May 31, 2023 •

edited

Loading

RedBeard0531 May 30, 2023

kraenhansen May 31, 2023 •

edited

Loading

RedBeard0531 Jun 2, 2023

RedBeard0531 Jun 6, 2023 •

edited

Loading

kraenhansen Jun 6, 2023

RedBeard0531 Jun 6, 2023

kraenhansen commented Jun 12, 2023

takameyer commented Jun 15, 2023

kraenhansen commented Jun 15, 2023

kraenhansen commented Jun 16, 2023 •

edited

Loading

	CompensatingWriteErrorType,
	CompensatingWriteErrorType as CompensatingWriteError,

		${GENERATED_OUTPUT_DIR}/native.d.mts
		${TYPESCRIPT_OUTPUT_DIR}/core.ts

		/packages/realm/generated/
		/packages/realm/src/generated/

CommonJS bundle (without Rollup) #5853

CommonJS bundle (without Rollup) #5853

Conversation

kraenhansen commented May 26, 2023 • edited Loading

What, How & Why?

kraenhansen commented May 26, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kraenhansen May 31, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kraenhansen May 31, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kraenhansen May 31, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

RedBeard0531 Jun 6, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kraenhansen commented Jun 12, 2023

takameyer commented Jun 15, 2023

kraenhansen commented Jun 15, 2023

kraenhansen commented Jun 16, 2023 • edited Loading

kraenhansen commented May 26, 2023 •

edited

Loading

kraenhansen May 31, 2023 •

edited

Loading

kraenhansen May 31, 2023 •

edited

Loading

kraenhansen May 31, 2023 •

edited

Loading

RedBeard0531 Jun 6, 2023 •

edited

Loading

kraenhansen commented Jun 16, 2023 •

edited

Loading