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

Upgrade to Realm Core v13.26.0 #6405

Merged
merged 7 commits into from
Jan 29, 2024
Merged

Upgrade to Realm Core v13.26.0 #6405

merged 7 commits into from
Jan 29, 2024

Conversation

kneth
Copy link
Contributor

@kneth kneth commented Jan 23, 2024

What, How & Why?

This is an upgrade of Realm Core. I have worked around the breaking change related to the implementation of new Realm.App() . We might want to discuss if we would like to expose app caching at a later time.

The new base URL functionality isn't exposed yet. It will be part of roaming project (yet to be decided).

Moreover, spec.yml needed a small update so the upgrade isn't as clean as I would like it.

This closes #6403

☑️ 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

@kraenhansen
Copy link
Member

kraenhansen commented Jan 24, 2024

The changes seem fine, but I suspect the @realm/react unit tests caused a crash.

It also likes like the integration tests error on the removal of users after a particular test

I think it's caused by this line

this.app = new Realm.App(appId);
and I think it needs to passe the baseUrl, which can be grabbed from here:
export const baseUrl = realmBaseUrl;

Copy link

coveralls-official bot commented Jan 24, 2024

Coverage Status

coverage: 85.346%. remained the same
when pulling 3b96f38 on kneth/realm-core-13.26.0
into 3fcac90 on main.

@kraenhansen
Copy link
Member

Have you had a chance to look into the crash from the @realm/react tests? It looks pretty deterministic.

Copy link
Contributor

@elle-j elle-j left a comment

Choose a reason for hiding this comment

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

Nice upgrade! Just have a comment about the base URL changes that would be good to address 🙂

Copy link
Contributor

Choose a reason for hiding this comment

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

I read the PR description, but from the Core spec.yml changes it does look like the base URL related changes are also there.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My impression is that it should be done later but I might be wrong.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah I see now that base_url was removed from the App and not AppConfig in the spec 👍

@cla-bot cla-bot bot added the cla: yes label Jan 25, 2024
@kneth
Copy link
Contributor Author

kneth commented Jan 25, 2024

Have you had a chance to look into the crash from the @realm/react tests?

It is someurl in https://github.com/realm/realm-js/blob/main/packages/realm-react/src/__tests__/AppProvider.test.tsx#L30 - it is a malformed URL, and modifying it to http://someurl makes the test pass.

@kneth kneth force-pushed the kneth/realm-core-13.26.0 branch from 81d6080 to 7e927a1 Compare January 25, 2024 13:46
CHANGELOG.md Outdated Show resolved Hide resolved
@kraenhansen
Copy link
Member

I've created #6441 to track fixing the crash 👍

@kneth kneth merged commit 7954133 into main Jan 29, 2024
33 checks passed
@kneth kneth deleted the kneth/realm-core-13.26.0 branch January 29, 2024 12:28
bimusiek pushed a commit to bimusiek/realm-js that referenced this pull request Mar 14, 2024
* Upgrade to Realm Core v13.26.0
* Use a well-formed URL in test
* Update CHANGELOG.md

---------

Co-authored-by: Kræn Hansen <[email protected]>
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 14, 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.

Upgrade to Realm Core 13.26.0
3 participants