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.

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

Printing calling stack frame when accessing global Realm #5628

Merged
merged 1 commit into from
Mar 28, 2023

Conversation

kraenhansen
Copy link
Member

What, How & Why?

This will improve the warning logged when someone access the global Realm from their code, by including the source-code frame reading it:

Your app is relying on a Realm global, which will be removed in realm-js v13,
 please update your code to ensure you import Realm via a named import:

 import { Realm } from "realm"; // For ES Modules
 const { Realm } = require("realm"); // For CommonJS
 
 This is happening at <anonymous> (/Users/kraen.hansen/Projects/realm-js/integration-tests/tests/src/tests/sync/realm.ts:384:26)

@kraenhansen kraenhansen self-assigned this Mar 23, 2023
@kraenhansen kraenhansen requested a review from takameyer March 23, 2023 16:18
@cla-bot cla-bot bot added the cla: yes label Mar 23, 2023
Copy link
Contributor

@takameyer takameyer left a comment

Choose a reason for hiding this comment

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

Cool warning. Will be great to get rid of this soon

@kraenhansen kraenhansen merged commit 9e6f73f into main Mar 28, 2023
@kraenhansen kraenhansen deleted the kh/printing-frame-on-global-realm branch March 28, 2023 09:37
Comment on lines +1970 to +1975
const stack = err instanceof Error ? err.stack || "" : "";
return stack
.split("\n")
.map((line) => line.trim())
.filter((line) => line.startsWith("at"))
.splice(1); // Skipping this function
Copy link
Contributor

Choose a reason for hiding this comment

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

Did you test that on all of node, hermes, and JSC? I know the stack format can differ between engines.

Copy link
Member Author

Choose a reason for hiding this comment

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

Nope ... I thought I wanted to add this with support for Node.js first and then add to it later (because it'll take time to test across runtime).

Anyways, thanks for the reminder. I'll likely test this tomorrow 🤞

Copy link
Member Author

@kraenhansen kraenhansen Mar 29, 2023

Choose a reason for hiding this comment

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

This is the output on iOS with Hermes (arguably not the most useful because source-maps aren't applied by default 🤔) - but also it doesn't blow up 🤷‍♂️ :

Your app is relying on a Realm global, which will be removed in realm-js v13, please update your code to ensure you import Realm via a named import:

import { Realm } from "realm"; // For ES Modules
const { Realm } = require("realm"); // For CommonJS

This is happening at anonymous (http://192.168.1.101:8081/index.bundle?platform=ios&dev=true&minify=false&modulesOnly=false&runModule=true&app=org.reactjs.native.example.RealmReactNativeTests:162740:5)

Copy link
Member Author

Choose a reason for hiding this comment

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

When revisiting this, I came up with a better strategy: #5660

@hpelitebook745G2
Copy link

hi @kraenhansen , I'm already using import Realm from 'realm', yet I'm still encountering this warning.

Context is I've implemented a Singleton class for Realm to ensure reusability throughout the application. Creating a separate instance for each component doesn't seem practical.

Or could you guide me to the recommended approach?

@kraenhansen
Copy link
Member Author

kraenhansen commented Oct 23, 2023

@hpelitebook745G2 I see you've created #6209 ... I'll answer there.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 15, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants