Add react-meteor-accounts

[CaptainN]

Adds the react-meteor-accounts package. Convenient react based alternatives for Meteor.user() and Meteor.userId().

It includes 8 facilities, 4 hooks and 4 HOCs.

• useUserId
• useUser
• withUserId
• withUser
• useLoggingIn
• useLoggingOut
• withLoggingIn
• withLoggingOut

TODOs:

• [x] Add tests
• [x] Flesh out Readme

[filipenevola]

@CaptainN how can we help here?

[CaptainN]

This just needs documentation and tests. (Or an executive decision to skip those)

[WilliamKelley]

I will contribute a PR to add readme documentation.

Additionally, why aren't these hooks written with useTracker / withTracker, respectively?

[CaptainN]

Not everyone who uses the react-meteor-data package will also use the accounts packages. I'd appreciate any contributions! Thanks!

[WilliamKelley]

@CaptainN I meant something a bit different -- why doesn't this package, react-meteor-accounts, depend on and use react-meteor-data? I assume all the considerations around using Tracker in React apply to this package and its utilities. I agree with the idea of keeping accounts utils separate 👍🏻.

I have created a PR for documentation, #347, with this branch as the base.

I have some additional opinions:

1. useUser / withUser should implement the signature of Meteor.user([options]), forwarding the optional options argument.
2. Hooks and HOC's should be defined in separate files, each lazy loaded so that any projects that only use hooks, or projects that want the optimization, don't have to include the HOC versions in their initial bundles.

I am willing to contribute these functionalities with go-ahead 👍🏻.

Edit: Also,

3. the project should have an index file, like react-meteor-data, that checks the React pkg version.
   • Side note: is there a reason that checkNpmVersions isn't used there, like detailed in the guide?

Edit 2:

4. The project should depend on Accounts ("accounts-base").
5. Hooks/HOC's for remaining Accounts data sources: loggingIn and loggingOut.

Edit 3:

6. Re-write in TS -- correct / improve signatures.
   • Component should be React.ComponentType not any.
   • ...UserId return type should be Meteor.User | null, identical to its data source signature.
   • ...User return type should be string | null, identical to its data source signature.

[CaptainN]

Ah! It doesn't rely on react-meteor-data because react-meteor-accounts doesn't have all the same considerations. ;-) One of the advantages of tailoring these hooks to just one specific API is they can be much simpler.

useTracker has to do a lot to support a wide range of different types of state. The thought here was that Meteor.user() and Meteor.userId() are relatively simpler APIs, and just don't need as much of that other work. That said, it does have some of the same considerations, and I haven't met them all. But I think it does meet all the most important considerations. This will work in a deterministic and reliable way.

As for splitting them up - I would bet the overhead from bundle/module boilerplate would be larger than the benefit. These hooks should be quite small when compiled. I bet the single js file is smaller than the package overhead already (roughly 918 bytes not-gzipped in the legacy bundle).

[CaptainN]

The reason I didn't use checkNpmVersions was because it used to add some bulk to the production bundle (around 8KB if memory serves) that I wanted to avoid. I don't know if it still does that.

[CaptainN]

Let me just respond to each one:

1. Cool idea! Let's do that! (Edit: it's possible that adding this crosses the line in complexity that it then makes sense to use useTracker. I'd be fine with it.)
2. Notes above. I'm not apposed, but would like to see a demonstrated need. I think the overhead would eat up the savings.
3. Agree that should be there.
4. Should it? Probably. There's probably not a case where it'd be worth it to split this in a different bundle. I'd say that's a good point.
5. Good catch! These would be great to add.
6. I'm happy to receive any TS improvements. Note though - the types aren't really used in production. They'll need to be ported to an @types project (or generated and submitted). It's the same with react-meteor-data.

Thanks for looking in to this! It's great to have some attention on these packages.

[WilliamKelley]

Meteor.user() and Meteor.userId() are relatively simpler APIs, and just don't need as much of that other work. That said, it does have some of the same considerations, and I haven't met them all. But I think it does meet all the most important considerations. This will work in a deterministic and reliable way.

Interesting, I'm pretty naive on what considerations aren't met without useTracker. But, if they're kept that way, I would only expect the unmet considerations to be stated in the README. May be a moot point.

Thanks for your responses to each point, I'll make some more contributions accordingly. Glad to help, this set of packages are very useful.

[WilliamKelley]

@CaptainN next PR in #348

[CaptainN]

Thanks to @WilliamKelley this is ready to go! We also got a couple of nice extra hook/HOC in useLoggingIn (and out, and hocs versions)

[filipenevola]

I miss a loading state for the user. Like I have in this package.

So we know that maybe the user is going to be there but it's not in Minimongo yet.

As you can see in this example.

I use it to avoid the content jumping in the UI during load.

What do you think @CaptainN @WilliamKelley ? It seems that you want to have just a single return in most of the hooks but handling the loading state is required in many cases to have a good UX.

[WilliamKelley]

It seems that you want to have just a single return in most of the hooks but ...

I'd justify their single return not as a principle of single return, but because the package is a reactive mirror-ing of Accounts data sources, not adding more sources.

I agree handling the loading state is important/useful. I think we should be mindful that altering the return signature will be weird for consumers expecting a reflection and complicate their adoption if they're migrating existing code that doesn't consider loading. So we should highlight this source's uniqueness in the docs, avoid further lock-in, and clarify the terminology.

For example,

"We define "loading" as meaning the user source may resolve to the current user's record or null. For example, the source will be "loading" during logout, during auto-log-in, and possibly other times when the user id is set, but the record is unavailable."

type UserNotLoading = [false, Meteor.User];
type UserLoading = [true, null];

function getUser(): UserNotLoading | UserLoading

I chose to return an array -- like React.useState -- so the user can use whatever names they want without the verbosity of renaming in object destructuring .

const [loading, user] = getUser();
const [loadingLoggedUser, loggedUser] = getUser();
// etc.

---

Just my two cents, feel free to take the direction y'all like.

[CaptainN]

I'd rather document the quirks, and not add another layer of variability to the API. From memory, one of the two methods (user() or userId) will return undefined during "loading" but then switch to null when the system has loaded, and the user is not logged in. I don't believe a falsy check to be sufficient in every case to detect the loading/initializing state.

I do think we should document whatever the exact behavior is though. There's no reason that custom useLoggedUser couldn't simply wrap useUser and apply the same logic.

[denihs]

Hi all,

Package published here.

Great job!

[StorytellerCZ]

@denihs why publish as mdg:react-meteor-accounts? I can get why not as react-accounts, but why under the mdg namespace? Was there some error?

[denihs]

Hi @StorytellerCZ,

Before publishing it, I talked with the team and decided to put the package under mdg. And as it isn't a core package, the author name, mdg in this case, needs to be on the name.

Is this bad? Do you have other suggestions? I think we can publish again, no problem.

[CaptainN]

mdg refers to a company that doesn't really exists anymore. If this is not going to be a core package, I'd suggest moving it out of this repo to maybe somewhere on community packages.

[denihs]

It makes total sense guys.

I was able to republish the package without the mdg: https://atmospherejs.com/meteor/react-meteor-accounts.

Now you just need to run meteor add react-meteor-accounts to install it.