Facts package unwarranted coupling with blaze/templating

[fgm]

Info	Details
Meteor versions	0.6.7-rc2 to 1.6.2-beta4
Last version without the problem	0.6.7-rc1 (package was not present yet)
OS	any
Reproduction	git clone https://github.com/meteor/meteor && cat packages/facts/package.js

Description of the problem

The Facts package provides two unrelated services:

• gathering the facts "hook" implementation results to expose them for further use without manually addressing the internal structures of Meteor (think facts-base)
• a simple way to expose these results in Blaze templates (think facts-ui)

Expected behavior

Have the UI be an optional package (facts-ui) or find a way to make it available only if templating is present, instead of requiring it. The list of dependencies should then just be:

facts@1.0.9                                   
├── ddp@1.4.0 (expanded above)                
├── mongo@1.3.1 (top level)                   
└── underscore@1.0.10                 

Actual behavior

All applications using this package bring in this whole list of dependencies:

facts@1.0.9                                   
├── ddp@1.4.0 (expanded above)                
├── mongo@1.3.1 (top level)                   
├─┬ templating@1.3.2                          
│ ├─┬ templating-compiler@1.3.3               
│ │ ├─┬ caching-html-compiler@1.1.2           
│ │ │ ├─┬ caching-compiler@1.1.9              
│ │ │ │ ├── ecmascript@0.9.0 (top level)      
│ │ │ │ └── random@1.0.10 (expanded above)    
│ │ │ ├── ecmascript@0.9.0 (top level)        
│ │ │ ├─┬ templating-tools@1.1.2              
│ │ │ │ ├── ecmascript@0.9.0 (top level)      
│ │ │ │ ├─┬ spacebars-compiler@1.1.3          
│ │ │ │ │ ├─┬ blaze-tools@1.0.10              
│ │ │ │ │ │ ├─┬ htmljs@1.0.11                 
│ │ │ │ │ │ │ └── deps@1.0.12 (expanded above)
│ │ │ │ │ │ └── underscore@1.0.10             
│ │ │ │ │ ├─┬ html-tools@1.0.11               
│ │ │ │ │ │ └── htmljs@1.0.11 (expanded above)
│ │ │ │ │ ├── htmljs@1.0.11 (expanded above)  
│ │ │ │ │ └── underscore@1.0.10               
│ │ │ │ └── underscore@1.0.10                 
│ │ │ └── underscore@1.0.10                   
│ │ ├── ecmascript@0.9.0 (top level)          
│ │ └── templating-tools@1.1.2 (expanded above)
│ └─┬ templating-runtime@1.3.2                
│   ├─┬ blaze@2.3.2                           
│   │ ├── check@1.2.5 (top level)             
│   │ ├── htmljs@1.0.11 (expanded above)      
│   │ ├── jquery@1.11.10 (expanded above)     
│   │ ├─┬ observe-sequence@1.0.16             
│   │ │ ├── diff-sequence@1.0.7 (expanded above)
│   │ │ ├── mongo-id@1.0.6 (expanded above)   
│   │ │ ├── random@1.0.10 (expanded above)    
│   │ │ ├── tracker@1.1.3                     
│   │ │ └── underscore@1.0.10                 
│   │ ├── ordered-dict@1.0.9 (expanded above) 
│   │ ├── reactive-var@1.0.11 (top level)     
│   │ ├── tracker@1.1.3                       
│   │ └── underscore@1.0.10                   
│   ├─┬ spacebars@1.0.15                      
│   │ ├── blaze@2.3.2 (expanded above)        
│   │ ├── htmljs@1.0.11 (expanded above)      
│   │ ├── observe-sequence@1.0.16 (expanded above)
│   │ ├── tracker@1.1.3                       
│   │ └── underscore@1.0.10                   
│   ├── templating-compiler@1.3.3 (expanded above)
│   └── underscore@1.0.10                     
└── underscore@1.0.10                 

Suggested fix

Convert to a wrapper package for compatibility (facts), use two separate packages (facts-base and facts-ui). I can submit a PR.

[hwillson]

Thanks @FGM - pull requests are welcome!

[abernix]

Thanks for drawing up this plan! I don't want to oversubscribe you, but if you learn anything during your investigation, or are inspired enough to also document the facts package while you're already in there, it would be great to improve your documentation on this! There's a long-standing issue open on the docs repository!

[fgm]

I sent a PR to show how I envision things. It's just a first draft to make sure we're on the same page (hence the [WIP]. I think it is best not to touch the docs for now, to keep the change as small as can be.

At this point, the PR passes the CI tests, and in an app (simple-todos), the server-side works, but not the ui. Checking.

[fgm]

PR working on our side, there are a few questions on the PR itself, though.

[convexset]

I guess this should be closed now.

I was thinking of adding a "set" based facts (to supplement the "increment" model). Any interest in that?

Basically, that would mean adding something like this:

Facts.setServerFact = function (pkg, fact, value) {
  if (!hasOwn.call(factsByPackage, pkg)) {
    factsByPackage[pkg] = {};
    factsByPackage[pkg][fact] = value;
    activeSubscriptions.forEach(function (sub) {
      sub.added(FACTS_COLLECTION, pkg, factsByPackage[pkg]);
    });
    return;
  }

  const packageFacts = factsByPackage[pkg];
  factsByPackage[pkg][fact] = value;
  const changedField = {};
  changedField[fact] = factsByPackage[pkg][fact];
  activeSubscriptions.forEach(function (sub) {
    sub.changed(FACTS_COLLECTION, pkg, changedField);
  });
};

I'm thinking of using that to provide information like:

import os from 'os';
const cpus = os.cpus();
{
    hostname: os.hostname(),
    osType: os.type(),
    osPlatform: os.platform(),
    osRelease: os.release(),
    arch: os.arch(),
    cpus,
}
// and in a setInterval...
{
    loadavg: os.loadavg().map(x => x / cpus.length),
    totalMem: os.totalmem(),
    freeMem: os.freemem(),
    upTime: os.uptime(),
}

[fgm]

This makes me think of the current https://github.com/fgm/meteor_server_info (from which this issue/PR originally came). Did you look at it ?

[convexset]

Just did. Looks nice albeit more complicated than what I would like.

Still, my objective is to be able to add "text facts". Already have added get/set methods on a local version of facts-base, but I do think those features are broadly applicable.

[fgm]

If you can think of any way to simplify it, I'm interested (on the issue queue over there, not here of course). I was mentioning it because of the type of facts you took as an example.

[abernix]

I believe the original feature here has been implemented in #9629. (Thank you, @fgm!) If there's desire for new additions to the facts package, we should open those up as new feature requests and discuss them there.

Thanks!