meteor-space / base

Foundation for Modular Application Architecture in Meteor.
MIT License
75 stars 5 forks source link

introduce better namespacing and class lookup #49

Closed DominikGuzei closed 8 years ago

DominikGuzei commented 8 years ago

This PR introduces a backward compatible way to define classes with fully qualified class path like this:

// Anonymous namespace -> not on global scope!
Space.namespace('My.custom')
FirstClass = Space.Object.extend('My.custom.FirstClass', {})
Space.resolvePath('My.custom.FirstClass') // == FirstClass

or like this:

// Namespace assigned to global scope
My.custom = Space.namespace('My.custom')
FirstClass = Space.Object.extend(My.custom, 'FirstClass', {})
Space.resolvePath('My.custom.FirstClass') // == FirstClass

which does a few things for us:

if you use Coffeescript with the class syntax, you still have to call @type 'My.custom.FirstClass' though since we have no control over the extension process there.

# Namespace assigned to global scope
My.custom = Space.namespace('My.custom')
class FirstClass extends Space.Object
  @type 'My.custom.FirstClass'
Space.resolvePath('My.custom.FirstClass') // == FirstClass
rhyslbw commented 8 years ago

What's the status here @darko-mijic ? Are we leaving this open while migrating the other packages and apps?

@Sanjo thoughts on this?

darko-mijic commented 8 years ago

We should migrate all dependent packages first. There are too much packages dependent on the develop branch of base. VOs would brake if we merge this now.

rhyslbw commented 8 years ago

@darko-mijic So it's not backwards compatible? Whatever the case be good to keep any specific discussion relating to a PR in the PR so it's easily reviewed

darko-mijic commented 8 years ago

Registering a type causes an error. Like Password.type('Password');. I think we should accept this breaking change and avoid introducing messy code for backwards compatibility. Especially since we have other breaking changes like serialization...

rhyslbw commented 8 years ago

It's important we get this merged now. Maybe we can divide up the packages to migrate?

darko-mijic commented 8 years ago

Readme needs to be updated and code snippets from Dominiks first comment added.