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

make sure defining modules doesn't have side effects #61

Closed
nilclass opened this issue Aug 14, 2012 · 3 comments
Closed

make sure defining modules doesn't have side effects #61

nilclass opened this issue Aug 14, 2012 · 3 comments

Comments

@nilclass
Copy link
Member

Right now defineModule calls the builder immediately, storing the module instance in an object. Once loadModule is called, the instance gets added to the remoteStorage object, and access is claimed on the corresponding paths. One reason to call the builder immediately within defineModule, is that this way module information (such as it's dataHints) is available without loading the module (which IMHO should be referred to as enabling the module, as it doesn't really load anything - the code has to be loaded already).

Now this brings trouble when the module wants to do initialization, that involves data. For example the calendar module calls privateBaseClient.sync('/'). This brings two problems:

  1. in this specific case, this creates an empty directory node "calendar/", which confuses my tests
  2. data is being accessed without claiming access to the directory. In reality this line probably won't do anything, because at the point where the module is defined, access cannot have been claimed, unless the user was just reloading the page, being connected already.

I can think of two ways to solve this:

  1. introduce a module initializer, that is called once the module is loaded
  2. only call the builder, when loading a module.

I prefer (1). Any thoughts on naming? initialize? onload? Anyway, it probably shouldn't be part of the exports but an additional key to the module's definition.

@nilclass
Copy link
Member Author

The issue referenced here should have been #61: ef1df8f

@nilclass
Copy link
Member Author

nilclass commented Sep 4, 2012

This issue may have become obsolete due to latest rework of loadModule. But I need to think about that a while longer...

@nilclass
Copy link
Member Author

I consider this obsolete now. In case things go wrong, let's rather open another issue.

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

No branches or pull requests

1 participant