-
Notifications
You must be signed in to change notification settings - Fork 585
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.
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
Replace "rollup" bundling with TypeScript project references #6492
Changes from 38 commits
6e88c2d
29bd876
27c977d
89d60b9
f7a1e68
34f67f0
a896a42
37665ed
6ed76e1
abae54d
a842394
3f8591a
1e9db90
e7ae6ea
d64d7a5
61f96bd
a5ebaeb
ceedf35
282ba72
297982a
2e7d5c7
03feb47
3190aa3
bfeb902
af2f46e
a29fb4c
c03de59
0f14343
b59d4f5
f5016c4
008f1e0
1b69b8f
ee6d53e
26f5586
a0a6971
769b0a0
429c64f
6c84b75
5574984
4cd729c
0f012aa
7b16a04
6309190
09aa48d
08dc7a7
d02a472
dc6a89b
e6818c5
5d830f0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,6 +4,7 @@ | |
"@tsconfig/node18/tsconfig.json", | ||
], | ||
"files": [ | ||
"src/node.ts" | ||
"src/node.ts", | ||
"src/types.ts" | ||
] | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,7 +2,8 @@ | |
"extends": "./tsconfig.node.base.json", | ||
"compilerOptions": { | ||
"outDir": "./dist/node-esm", | ||
"module": "ES2022" | ||
"module": "es2022", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So you want to prepare for top-level |
||
"moduleResolution": "Bundler" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. According to the docs, it should be There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Either is fine, this was suggested by VSCode - it seems this property isn't case-sensitive. |
||
}, | ||
"references": [ | ||
{ "path": "tsconfig.types.json" } | ||
|
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -17,6 +17,7 @@ | |||||
//////////////////////////////////////////////////////////////////////////// | ||||||
|
||||||
import React, { createContext, useContext, useEffect, useState } from "react"; | ||||||
import Realm from "realm"; | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You may be able to only import the type here.
Suggested change
|
||||||
|
||||||
import { useApp } from "./AppProvider"; | ||||||
|
||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -137,35 +137,7 @@ list(FILTER SDK_TS_FILES EXCLUDE REGEX "templates/[^/]*\.ts$") | |
|
||
set(JS_SPEC_FILE ${SDK_DIR}/bindgen/js_spec.yml) | ||
set(JS_OPT_IN_FILE ${SDK_DIR}/bindgen/js_opt_in_spec.yml) | ||
set(TYPESCRIPT_OUTPUT_DIR ${SDK_DIR}/generated/ts) | ||
|
||
set(GENERATED_TS_FILES | ||
${TYPESCRIPT_OUTPUT_DIR}/native.d.mts | ||
${TYPESCRIPT_OUTPUT_DIR}/core.ts | ||
) | ||
bindgen( | ||
TEMPLATE ${SDK_DIR}/bindgen/src/templates/typescript.ts | ||
OUTPUTS ${GENERATED_TS_FILES} | ||
OUTDIR ${TYPESCRIPT_OUTPUT_DIR} | ||
SPECS ${JS_SPEC_FILE} | ||
OPTIN ${JS_OPT_IN_FILE} | ||
SOURCES ${SDK_TS_FILES} | ||
) | ||
|
||
set(GENERATED_JS_FILES | ||
${TYPESCRIPT_OUTPUT_DIR}/native-node.mjs | ||
${TYPESCRIPT_OUTPUT_DIR}/native-react-native.mjs | ||
) | ||
bindgen( | ||
TEMPLATE ${SDK_DIR}/bindgen/src/templates/node-wrapper.ts | ||
OUTPUTS ${GENERATED_JS_FILES} | ||
OUTDIR ${TYPESCRIPT_OUTPUT_DIR} | ||
SPECS ${JS_SPEC_FILE} | ||
OPTIN ${JS_OPT_IN_FILE} | ||
SOURCES ${SDK_TS_FILES} | ||
) | ||
|
||
add_custom_target(TypeScript ALL DEPENDS ${GENERATED_TS_FILES} ${GENERATED_JS_FILES}) | ||
Comment on lines
-140
to
-168
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I suggest driving this entirely from Wireit, to have a single point of configuration that doesn't diverge. |
||
set(NODE_OUTPUT_DIR ${SDK_DIR}/binding/build) | ||
|
||
if(DEFINED CMAKE_JS_VERSION) | ||
include(node.cmake) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -10,8 +10,8 @@ set_target_properties(realm-js-node PROPERTIES | |
|
||
set_target_properties(realm-js-node PROPERTIES | ||
# Need a dummy generator expression to avoid adding in the config name | ||
LIBRARY_OUTPUT_DIRECTORY "${TYPESCRIPT_OUTPUT_DIR}/$<0:dummy_genex>" | ||
RUNTIME_OUTPUT_DIRECTORY "${TYPESCRIPT_OUTPUT_DIR}/$<0:dummy_genex>" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This got renamed since Cmake is no longer generating TypeScript files. |
||
LIBRARY_OUTPUT_DIRECTORY "${NODE_OUTPUT_DIR}/$<0:dummy_genex>" | ||
RUNTIME_OUTPUT_DIRECTORY "${NODE_OUTPUT_DIR}/$<0:dummy_genex>" | ||
) | ||
|
||
if(WIN32) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,8 +1,5 @@ | ||
{ | ||
"name": "@realm/js-binding", | ||
"name": "@realm/js-bindgen", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is not the binding, but the JS specific tool / package used to generate it. |
||
"private": true, | ||
"type": "module", | ||
"workspaces": [ | ||
"packages/realm/bindgen/vendor/realm-core/" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This was pointing nowhere. I know Mathias wanted to keep it around as an example on how to integrate this in another SDK, but that will probably be the least of their worries 🙂 |
||
] | ||
"type": "module" | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
According to the docs, it should be
node16
.