proposal: getting rid of claimAccess and displayWidget

[michielbdejong]

i think we could simplify the basic getting-started experience by adding some default behaviour. say someone includes for instance:

    <script src="remoteStorage.js"></script>
    <script src="documents.js"></script>
    <script src="contacts-ro.js"></script>

into their app (where '-ro' stands for read-only). Then the whole following contraption is currently required, but imho superfluous - adding:

    <div id="remotestorage-connect"></div>

in the body, and then you have to do:

    remoteStorage.claimAccess({
      documents: 'rw',
      contacts: 'r'
    }).then(function() {
      remoteStorage.displayWidget('remotestorage-connect');
    });

if people don't like publishing one default and one -ro version of each module, then as an alternative we can keep claimAccess for the case where you want to have some of the modules you require as read-only

[raucao]

The simpler the better. I like.

[jancborchardt]

This doesn’t seem to simplify anything. Access management should be done in one place like it is now, not via complexification of the module files.

[michielbdejong]

@jancborchardt I think what you're saying is that my proposal hides too much of the mechanics. but all current apps use displayWidget('remotestorage-connect'), so we can make that the default. Also, 8 out of 9 current apps claim read-write access to exactly one module; only the music app claims read-only access.

Making access management explicit as it is now may help the developer feel like they know what's going on, but if it's always the same hard-to-memorize ritual of essentially meaningless lines of code, then it does not actually help for anything.

[michielbdejong]

see also https://github.com/vcuculo/ghost-unhosted-webrtc/issues/5#issuecomment-12484745 for a real-world example

[xMartin]

Isn't this two separate issues? It would be nice to make displaying the widget optional (still the default, at least concerning docs) so would that go well with getting rid of the call?

See also #211.

[jancborchardt]

@michielbdejong that’s not what I’m saying, I’m saying it’s too damn confusing. Especially when 8 out of 9 apps claim read-write anyway why make it convoluted?

[raucao]

@xMartin Good point! The separation issue should probably be tackled first, as that will influence the setup code.

@jancborchardt I think what @michielbdejong is trying to achieve is removing as much boilerplate code as possible. That should always be a goal, and if you find leaving it away is too confusing, please tell us how you'd simplify the current version instead. At the moment it's more convoluted than it was before, due to having to use the promise manually. The current boilerplate also doesn't make a lot of sense from an outside point of view, as it isn't apparent what claimAccess does and why you need to wait for it to display something.

[michielbdejong]

how about a setOption command. That's how CodeMirror does it, and i like it a lot. for us it would be something like:

    remoteStorage.setOption('widget', false);//default value: 'remotestorage-connect'
    remoteStorage.setOption('access', { documents: 'rw', contacts: 'r' });//default value: 'rw' for all modules you load

[silverbucket]

I like the idea of having less public facing functions and more configuration via. parameters. That way it's easier for developers to find what they need and not go hunting for function names. So this sounds great to me, maybe we could make more things use the setOption() function as well.

On Mon, Jan 21, 2013 at 11:10 PM, Michiel@unhosted <[email protected]

wrote:

how about a setOption command. That's how CodeMirror does it, and i like it a lot. for us it would be something like:

remoteStorage.setOption('widget', false);//default value: 'remotestorage-connect'
remoteStorage.setOption('access' { documents: 'rw', contacts: 'r' });//default value: 'rw' for all modules you load

— Reply to this email directly or view it on GitHubhttps://github.com/RemoteStorage/remoteStorage.js/issues/204#issuecomment-12504302.

[raucao]

Yer, like setOption('sync', category, false)! ;)

[michielbdejong]

it would also actually make sense to me if it were

    remoteStorage.module.claimAccess('rw')

instead of

    remoteStorage.claimAccess({ module: 'rw' })

since it is the module that claims access to its storage area, not the app.

the app only gets access to the module, and the module is the one who, in turn, gets read or read-write access to the actual storage.

in that light, it might also make sense to merge claimAccess into use(), so then it would be something like

    privateClient.use('', { readOnly: true, treeOnly: true });

[nilclass]

for reference, in #221 @skddc suggested remoteStorage.init.

[silverbucket]

ping @michielbdejong - Do you think this issue is still relevant, or can we close it?

[michielbdejong]

Yeah, at this point it doesn't make much sense to change the API now that we've already been using it so long.

[raucao]

now that we've already been using it so long.

If that is the sole reason, I think we should keep it open. We don't have to break the API right away, but I still think we can improve it and we're not on 1.0 yet. We can always have backwards compat and deprecration warnings.

[raucao]

Current status here ist that in 1.0.x, there is no widget anymore, because it's now a seperate add-on library (aiming to use only public API so devs can easily implement their own UI).

And as for the other half of the issue, it is now required to use the RemoteStorage constructor to initialize the instance. Also, rs.claimAccess() has been changed to rs.access.claim(). That means we can now add an access/permissions map to the constructor's config options argument in order to allow claiming it directly when creating the instance.

Whoever's interested in implementing this, please self-assign the issue, or add a comment here that you started working on it. Thanks!