-
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
useUser
fixes for Realm React
#6206
Conversation
039cd2d
to
5656140
Compare
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.
Awesome to get this fix in! LGTM when this PR is merged into the current one 🙂
@@ -16,7 +16,7 @@ | |||
}, | |||
"wireit": { | |||
"bundle": { | |||
"command": "rollup --config", | |||
"command": "rollup --config && tsc -p tsconfig.types.json", |
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.
Should this be a separate types
command? Since technically this isn't bundling the types now.
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.
As our ci assumes a single command for preparing packages for release, I would be hesitant to put this in a separate command.
Otherwise, we can make a follow up PR which adds a types
command to every package (or simply build
, which contains bundle
and types
) and update our workflows to call this as well.
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.
@kraenhansen I put this in our next roundtable. I think you bring up a good point.
My suggestion is to create a script in each of our mono-repo packages called prepack
, which can include whatever scripts it needs to prepare for being consumed by another project. This could include bundle
, generateTypes
, build
, etc.
Then the CI workflows would be updated to run this command instead of bundle
.
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.
It was the idea that these inter-package task dependencies should be configured via wireit. It's much more capable as it'll cache outputs and potentially execute tasks in parallel.
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.
I don't disagree, I just want to unify on a script that specifically deals with preparing for release.
* Used a different type declaration generation method which solved typing issues * Updated `UserProvider` to always update the user reference on user events * Updated connection example to include a delete user function to test this feature
Co-authored-by: LJ <[email protected]>
792328c
to
6f07e03
Compare
Co-authored-by: LJ <[email protected]>
What, How & Why?
UserProvider
to always update the user reference on user eventsThis closes #6196 and #6186
☑️ ToDos
Compatibility
label is updated or copied from previous entryCOMPATIBILITY.md
package.json
s (if updating internal packages)Breaking
label has been applied or is not necessary