Update to use bv-ui-core modules in place of local modules. #35
Conversation
| // | ||
| // Other code may have a reference to the object, so we can't do anything | ||
| // more drastic such as replacing it. | ||
| if (!(global[name] instanceof Namespace)) { |
There was a problem hiding this comment.
My first instinct is to error out here. If window.APP is already defined, for example, but not a Namespace, then who knows what expectations the originator has about it? For example, window.APP.name and other properties are about to be assigned and might overwrite existing properties. The whole thing could even be deleted.
That said, it may be fair to make this assumption a part of the deal to namespacer users if it buys some convenience. Is there a specific scenario we care about in which it does?
There was a problem hiding this comment.
The scenario we're accommodating here is that other, older BV code created window.APP = {}; as the shared namespace. We then have to play nice with that.
The method used below to decorate that object literal is probably terrible and would need improvement in the bv-ui-core version of this.
|
I really like this approach. It definitely looks like something that's |
reason-bv
force-pushed
the
kernel-refactor
branch
from
99cd523 to
4c24789
Compare
|
For added clarity, the automated tests are failing pending a |
reason-bv
changed the title
| * on global.NS using the application name, and exposes a render | ||
| * method on an object there. | ||
| * A module for registering an application. It creates a property on global.NS | ||
| * using the application name, and exposes a render method on an object there. |
There was a problem hiding this comment.
Is this still accurate? I don't see the render method provided.
There was a problem hiding this comment.
Nevermind, I think I mis-read this. But it does seem weird to mention the render method here when it isn't actually provided here. A more generic comment may still be desirable even if this one is accurate.
reason-bv
force-pushed
the
kernel-refactor
branch
from
4c24789 to
8da0863
Compare
|
@rmurphey Good catch, rewrote those few lines of docs. |
|
I'm otherwise good with this. Probably bears mentioning here that you opened #36. |
|
👍 |
This PR updates code and documentation for the use of bv-ui-core modules rather than scoutfile modules.