Clean and reasonable project structure

[johnSamilin]

Refactored data sources component to make it a separate page
according to issue #633

[chrisknoll]

Have you tried the component binding without a container element?
See: http://knockoutjs.com/documentation/component-binding.html

Like so:

<!-- ko component: {
    name: $component.currentReport().name,
    params: { context: $component }
} -->
<!-- /ko -->

The component will reload based on the value of currentReport().name (or you can introduce a new property on the report called 'componentName' in the case that you want to keep the name of the report separate from the name of the component for this report.

This would replace the need for the set of <!-- ko if: .... --> statements you have.

[johnSamilin]

I never knew there's such binding, thanks for the hint!

[chrisknoll]

Hello everyone,
Obviously, there's a lot to unpack in this PR and I think instead of creating a large list of questions that may be unrelated, I'll start with some top level questions/comments that we can cover, and then get down into more specific questions. It will be difficult to keep all the separate threads organized under this one 'pr' thread, but I'll try to completely cover one topic before moving into the next one.

So, first question/comment: using packages and what it means for dynamic loading of pages:

I like packages, and I enjoy how they can take a group of related files together and let you load a unit of functionality together without having to mess with a bunch of paths configs; you just define a package name, point it to an entry module (by default: main.js) and you're done.

The problem I see with how packages is being used here (for the package: 'pages'), is that as we add more and more pages under the 'pages' package, the application will pre-load all the dependencies regardless if the user needs them immedately. What we'll wind up with is that doing a requre(['pages']) will load the entire app, when we want to lazy load the application. Currently, we lazy-load modules based on the route accessed: within the route handler, we do require(["pageComponent"], function() {...});. As the user navigates thorough the app, more of the application is loaded.

Should we think about 'package-per-page' mindset, or not use packages all together so that we can lazy load only the pages that we immediately need?

[johnSamilin]

Hi @chrisknoll
the only thing that the app will pre-load is routes for all the pages. The page itself is still being lazy loaded at the time user navigates to the corresponding route, so I'd say there's no need to have a package per each page

[chrisknoll]

Hi, @johnSamilin,
I stand corrected, you're absolutely right, I misread the main.js file, only the routes are being loaded:

define(
  (require, exports) => {
    const buildRoutes = require('./routes');

    return {
      buildRoutes,
    };
  }
);

Very cool, it makes sense that routes need to be loaded up front, otherwise the routing library won't know what to do with a given path. Do you think it's possible, in the future, that we leverage a routing library that can register sub-routes at runtime based on a root route? The reason I ask: data-sources routs get loaded, then when you access /data-sources route, it triggers the require() to load in all the reports. Could we get to yet another level of lazy loading, by only loading the specific sub report when requested? So if someone goes to /datasources/condition/{concept_id} (which, we don't actually have routes for this, I know) the routing library would be like:
handle /datasources (it was registered at the app.js level by loading pages)
...handling /datasources leads to a require() that loads immediate child routes (condition will be one of them)
....the router than handles the next part of the path 'condition'...
......the route handler for condition requires() some dependent report assets for condition reports and also loads up the route configuration to handle the drilldown to a concept_id
.......the route handler for /datsources/condition/{id} now exists so it will handler that route.

I'm not suggesting we make any changes to support this functionality (since we're just talking about project structure here) but since we are handling routes a little differently, I wanted to put the question on the table about what you thought about dynamic building of route tables on the fly...with the extra twist that as a route is being processed, the child routes could be added to the route table so that the required assets get loaded as the routes get initially discovered.

[chrisknoll]

Re: the pages package's main.js

In the main.js for the pages package, we're loading up ./data-sources/index module, which in turn loads up the routes and returns them. Is there any reason we could skip that intermediate index.js and just load the routes directly in main.js? Then all that the purpose of main.js is to establish the routes that are necessary for the package, and then as the routes are invoked, the sub-resources will be pulled in.

[chrisknoll]

Hello, everyone,
Putting the question about handling routes aside and the intermediate index.js file, this all looks really good. So I'm happy to put approval on this, and leave it to you guys to decide how you want to proceed with those open questions.

If you do proceed with the PR as-is: please squash those commits down into a 'folder restructure - Part 1' commit. Thank you!