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

[WIP] New hooks #298

Closed
wants to merge 25 commits into from
Closed
Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
d8ad782
Use React.DependencyList instead of Array<any>
CaptainN Oct 19, 2020
c4c9f62
Add an accounts hook package
CaptainN Oct 21, 2020
b1281d1
Add react-meteor-state hook package
CaptainN Oct 21, 2020
5932980
Add react-mongo slim hooks package
CaptainN Oct 21, 2020
7460169
Always render on commit and apply feedback on deps
CaptainN Nov 1, 2020
0b99ffe
Fix subscriptions not closed when stopping computation
rijk Nov 2, 2020
d94b254
Merge pull request #302 from rijk/new-hooks
CaptainN Nov 2, 2020
98d80d2
Voidable sub factory, reactive ready, and a Readme
CaptainN Nov 3, 2020
b9404da
Use factoryless useSubscription other improvements
CaptainN Nov 8, 2020
e075fba
Help Typescript keep track of type of refs
CaptainN Nov 9, 2020
0d9f3ba
Fix ready() doesn't update, and update react-mongo type def
CaptainN Nov 10, 2020
e11f8bc
Remove unneeded complexity in useSubscription
CaptainN Nov 11, 2020
d5b5f8e
Use memoized refs.params to reduce useEffect churn
CaptainN Nov 18, 2020
f59dd39
Make useCursor memoize fetch results
CaptainN Nov 20, 2020
ba67ebb
First run after deps change returns the right data
CaptainN Nov 21, 2020
37ff546
Fix initial data after deps change behavior
CaptainN Nov 22, 2020
783dad9
Add useFind, useFindOne, useCount
CaptainN Nov 23, 2020
9aec19d
Make useFindOne and useCount accept factory methods
CaptainN Nov 24, 2020
754953a
Update useTracker cleaner impl, and clean up tests
CaptainN Nov 26, 2020
9fa3df9
Apply feedback to useFind and useSubscription
CaptainN Nov 26, 2020
9ed323f
Check for cursor on server too
CaptainN Nov 29, 2020
9dabef9
Clean up type notation, pass comp to reactiveFn
CaptainN Nov 29, 2020
b06c4cd
Fix failing resubscribe tests by forcing full flush cylce
CaptainN Dec 1, 2020
6d056ec
Improve signal to noice of resubscribe tests
CaptainN Dec 1, 2020
51b2f38
Update packages and add typescript gen script
CaptainN Dec 1, 2020
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions packages/react-accounts/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
meteor/react-accounts
=====================

A couple of simple hooks for accessing Meteor Accounts state.
430 changes: 430 additions & 0 deletions packages/react-accounts/package-lock.json

Large diffs are not rendered by default.

17 changes: 17 additions & 0 deletions packages/react-accounts/package.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
/* global Package */

Package.describe({
name: 'react-accounts',
summary: 'React hook for reactively tracking Meteor data',
version: '0.9.0',
Copy link
Collaborator

Choose a reason for hiding this comment

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

We can probably start at v1.0.0

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I'll make that change in a new PR

documentation: 'README.md',
git: 'https://github.com/meteor/react-packages',
});

Package.onUse(function (api) {
api.versionsFrom('1.10');
Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's start at least at 1.12

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The reason for this is simply that I can't get the tests to run on anything newer than Meteor 1.10...

api.use('tracker');
api.use('typescript');

api.mainModule('react-accounts.ts', ['client', 'server'], { lazy: true });
});
15 changes: 15 additions & 0 deletions packages/react-accounts/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
{
"name": "meteor-react-accounts",
"scripts": {
"make-types": "npx typescript *.ts --declaration --emitDeclarationOnly --outDir types"
},
"dependencies": {
"@testing-library/react": "^10.0.2",
CaptainN marked this conversation as resolved.
Show resolved Hide resolved
"@types/meteor": "^1.4.42",
"@types/react": "^16.9.34",
"react": "16.13.1",
"react-dom": "16.13.1",
"react-test-renderer": "16.13.1",
"typescript": "^4.0.3"
}
}
29 changes: 29 additions & 0 deletions packages/react-accounts/react-accounts.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
import { Meteor } from 'meteor/meteor'
import { Tracker } from 'meteor/tracker'
import { useState, useEffect } from 'react'

export const useUserId = () => {

Choose a reason for hiding this comment

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

I just watched your talk in Impact Meteor 2020 and when you first introduced this guy I thought it would be global managed. I'm wondering that we are opening a computation every time we need userId in a component. I thought zustand would better fit this problem, although it might not be necessary because I don't know the impacts of openings many computations. What do you think? It could be applied in others states as well.

Copy link
Contributor Author

@CaptainN CaptainN Dec 29, 2020

Choose a reason for hiding this comment

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

Computations are incredibly cheap in practice. In Blaze they are the backbone of the entire system. In Meteor, for years when using withTracker it would actively create and destroy a computation on every react render (it still does, as does useTracker when no deps are provided). It's basically adding one entry to an array internally, for the reactive source. Very cheap.

Still, to recycle some of the more expensive reactive sources (not the computation itself) like an expensive reactive subscription, you can always hoist the computation up your react tree (er, or down to root), and then use the context API to feed the result of that one computation to mulitple hooks. I showed how to do that in the hook introduction blog post (here is the gist). Be aware though, this can introduce some subtle timing issues with reactivity and the React lifecycle. It's easier (and cheap) to just use a lot of computations. ;-)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

One point I mean to stress whenever we ship these, is they are not solving a problem with the current HOC/hook - useTracker is fast as is, and we'll still recommend its use with Collection.findOne for example. We/I simply couldn't come up with anything better for that.

These new hooks are the result of some exploratory effort, to see if we can gain some advantage by implementing things in a more custom way. What I found was that some of my earlier assumptions (which I did express in the Impact Meteor 2020 preso) did not pan out - just exposing a cursor was actually quite a bit slower and less efficient than useTracker...

The main advancements we did achieve here are in 2 very specific cases (so far anyway, there may be others when we/I get to them). useSubscription is more optimized for subscriptions in particular. In practice, it probably won't amount to much in terms of gains, but it has a cleaner API than useTracker for subscriptions in particular. The second one is the big win - useFind is now immutable, and doesn't require the subscription to be mixed in with the cursor. This has the potential to open up new use cases for high performance reactivity in lists over what useTracker and withTracker could deliver. I say "has the potential" because these statements have not been well tested, and there are likely tradeoffs here (as with any performance optimizations). But really, the main advantage of these 2 hooks is mostly that they (arguably) improve developer experience, through a more tailored/integrated feeling API between Meteor and React, and I always love that type of improvement.

Along the way, I also discovered some improvements we should make to useTracker which have been submitted as a separate PR.

const [userId, setUserId] = useState(Meteor.userId())
CaptainN marked this conversation as resolved.
Show resolved Hide resolved
useEffect(() => {
const computation = Tracker.autorun(() => {
setUserId(Meteor.userId())
})
return () => {
computation.stop()
}
}, [])
return userId
}

export const useUser = () => {
const [user, setUser] = useState(Meteor.user())
useEffect(() => {
const computation = Tracker.autorun(() => {
setUser(Meteor.user())
})
return () => {
computation.stop()
}
}, [])
return user
}
3 changes: 3 additions & 0 deletions packages/react-accounts/types/react-accounts.d.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
import { Meteor } from 'meteor/meteor';
export declare const useUserId: () => string;
export declare const useUser: () => Meteor.User;
7 changes: 3 additions & 4 deletions packages/react-meteor-data/useTracker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,13 +39,12 @@ type ComputationHandler = (c: Tracker.Computation) => () => void | void;
type TrackerRefs = {
reactiveFn: ReactiveFn;
computationHandler?: ComputationHandler;
deps?: Array<any>;
deps?: React.DependencyList;
computation?: Tracker.Computation;
isMounted: boolean;
disposeId?: ReturnType<typeof setTimeout>;
trackerData: any;
computationCleanup?: () => void;
trackerCount?: number
}

// The follow functions were hoisted out of the closure to reduce allocations.
Expand Down Expand Up @@ -133,7 +132,7 @@ const tracked = (c: Tracker.Computation, refs: TrackerRefs, forceUpdate: Functio
};

interface useTrackerSignature {
(reactiveFn: ReactiveFn, deps?: null | Array<any>, computationHandler?: ComputationHandler): any
(reactiveFn: ReactiveFn, deps?: null | React.DependencyList, computationHandler?: ComputationHandler): any
}

const useTrackerNoDeps: useTrackerSignature = (reactiveFn, deps = null, computationHandler) => {
Expand Down Expand Up @@ -184,7 +183,7 @@ const useTrackerNoDeps: useTrackerSignature = (reactiveFn, deps = null, computat
return refs.trackerData;
}

const useTrackerWithDeps: useTrackerSignature = (reactiveFn, deps: Array<any>, computationHandler) => {
const useTrackerWithDeps: useTrackerSignature = (reactiveFn, deps: React.DependencyList, computationHandler) => {
const { current: refs } = useRef<TrackerRefs>({
reactiveFn,
isMounted: false,
Expand Down
4 changes: 4 additions & 0 deletions packages/react-meteor-state/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
meteor/react-meteor-state
=========================

A state hook, with an API similar to React's useState, which provides data persistence between page reloads using hte Meteor reload package.
Copy link
Collaborator

Choose a reason for hiding this comment

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

typo in hot

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That whole readme needs to be written. :)

Loading