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

Release 2.4.0 #113

Merged
merged 42 commits into from
Mar 22, 2023
Merged

Release 2.4.0 #113

merged 42 commits into from
Mar 22, 2023

Conversation

jankapunkt
Copy link
Member

This merges the current dev state into master, making 2.4.0 the new main HEAD.

ToyboxZach and others added 12 commits February 17, 2023 11:03
…y better

Merge pull request #111 from ToyboxZach/Merge

Restructure how we handle the Tracker, instead of a global listen to any change make the listens happen on specific queries and collections. Also fixes some bugs where we would generate way too many instances of subscriptions.

This also fixes the log in flow so the series of loggingIn -> LoggedIN is consistent and doesn't allow the package to ever auto log you out as that should be handled by the owning app just like in base meteor.

Every time you register a callback it gets added to a generic "changed" callback, that means that every single callback needs to be called on every single change to a collection which causes a huge hold up on the UI thread.

To make it even worst the subscriptions don't get cleaned up when you resubscribe so every time a "withTracker" or "useTracker" was called you would get stacked subscriptions. This then would lead to 100 plus subscriptions for a single component after some time which leads to bigger and bigger slow downs.

My PR separates that out as much as possible and makes sure that you only have as many subscriptions as you actually need. It is a pretty hefty change and I don't personally use and local collections in my app so those are going to be pretty poorly tested, but if other people want to take my change as a jumping off point I think its a pretty good point for server only subscriptions.

This also fixes up weirdness with the logging in and order of events where you would have an invalid user state or it would force you to logout because of bad internet

Likely fixes these erorrs:
#75
#58

and maybe
#79
@jankapunkt
Copy link
Member Author

@TheRealNate with this release we will have 0 vulnerabilities, a pretty good coverage and even a few bugs fixed due to #111

Coverage report:

-------------------------|---------|----------|---------|---------|----------------------
File                     | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s    
-------------------------|---------|----------|---------|---------|----------------------
All files                |   85.67 |    80.69 |   81.54 |   86.33 |                      
 helpers                 |   63.63 |      100 |   33.33 |      75 |                      
  reactNativeBindings.js |   63.63 |      100 |   33.33 |      75 | 14-16                
 lib                     |   93.01 |    88.77 |   96.87 |   93.15 |                      
  Random.js              |     100 |      100 |     100 |     100 |                      
  ddp.js                 |   86.15 |     87.5 |   89.47 |    87.5 | 43-59                
  mongo-id.js            |     100 |      100 |     100 |     100 |                      
  queue.js               |     100 |      100 |     100 |     100 |                      
  sha256.js              |   92.59 |       40 |     100 |      92 | 206-212              
  socket.js              |     100 |      100 |     100 |     100 |                      
  utils.js               |   84.61 |       88 |     100 |   84.61 | 36,69,75-76          
 src                     |   81.71 |    77.51 |   73.26 |   82.31 |                      
  Call.js                |     100 |       75 |     100 |     100 | 5                    
  Collection.js          |   76.77 |       72 |   73.68 |   77.85 | ...3,262,291,302-317 
  Data.js                |   23.68 |    35.71 |    6.25 |   23.68 | 15-81,92-93          
  ReactiveDict.js        |     100 |      100 |     100 |     100 |                      
  Tracker.js             |      92 |    81.63 |   94.44 |   93.15 | ...4,411,512,534,620 
-------------------------|---------|----------|---------|---------|----------------------

@TheRealNate
Copy link
Collaborator

Seems like it will be a big help! Approved, LGTM.

@jankapunkt jankapunkt merged commit 3f4cd96 into master Mar 22, 2023
@jankapunkt
Copy link
Member Author

It's merged but I still can't publish to NPM registry, can you do that or add me to the ord on NPM (I have 2FA active on NPM)

@TheRealNate
Copy link
Collaborator

@jankapunkt sure can do, same username?

@jankapunkt
Copy link
Member Author

@TheRealNate thanks I just got the invitation and will publish now

@jankapunkt
Copy link
Member Author

Hm seems to be revoked, anyway my account is https://www.npmjs.com/~jkuester

@TheRealNate
Copy link
Collaborator

Added!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants