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

Angular2-localstorage repository's future #80

Open
DanielKucal opened this issue Mar 14, 2017 · 14 comments
Open

Angular2-localstorage repository's future #80

DanielKucal opened this issue Mar 14, 2017 · 14 comments

Comments

@DanielKucal
Copy link

DanielKucal commented Mar 14, 2017

Hi, at the beginning I have to say I really appreciate functionality this project provides. It would be a great loss if this project had been died and it loses much in the time it's not maintained. As we need it in @zoomsphere, I made a fork in zoomsphere/ngx-store that includes the following changes:

  • update allowing to use it with Angular2 Final,
  • replaced deprecated typings by @types,
  • improved Webstorage.clear() method that now removes items created by decorators only,
  • added support for Array methods,
  • has configurable config (for now containing just storage key prefix, but it'll be more),
  • added .save() method for objects stored in session/local storage to force save,
  • updated README.md,
  • has defined inner interfaces,
  • new build,
  • and probably something more I forgot. 😄

It's now available on npm install ngx-store --save

To get down to brass tacks, we can maintain this repository under our organization brand. My idea for its development is to add support for any stored data manipulations, create unit tests, add option to set data expiration time and encoding / minifying of stored data (configurable).
I know pretty well this project's code and I've also already contributed to popular A2-related repositories like ng2-bootstrap, ng2-select, ng2-file-upload, keep angular2-rest up-to-date and our fork of is the most developed one, so I think this is either the most natural and serious maintenance proposition.
I'm looking forward to hear what do you think about it, @marcj. Thanks in advance!

@DanielKucal DanielKucal changed the title Repository future Angular2-localstorage repository's future Mar 14, 2017
@ssidorchik
Copy link

hi @DanielKucal. Very nice enchantment over the original library.
Unfortunately when try to make AOT build it fails with error - ERROR in Unexpected value 'WebStorageModule in /Users/sergey/Projects/test/test-acli/node_modules/angular2-localstorage/dist/index.d.ts' imported by the module 'AppModule in /Users/sergey/Projects/test/test-acli/src/app/app.module.ts'.
Could you please let me know if you have plans to support AOT?

@zilions
Copy link

zilions commented Mar 22, 2017

Also having the same issue as above

@DanielKucal
Copy link
Author

Hi @ssidorchick and @zilions, thank you for your question. We don't use AoT compiliation for now, but probably will in the future, so I want it to work with both compilers. Unfortunately, I'm currently unable to investigate this error, so pull request or tips will be welcome.

@DanielKucal
Copy link
Author

@ssidorchick @zilions @MarcelMalik good news guys, AoT compilation should now pass on aot branch: https://github.com/zoomsphere/angular2-localstorage/tree/aot
Please try it and let me know if it works for you. You can also do it via npm install ngx-store.
@marcj, any chances this project will be transferred?

@HarelM
Copy link

HarelM commented May 19, 2017

I'm using @zoomsphere branch and since this project needs a maintainer I don't see why it shouldn't be them (also since their branch support angular 4 I think the least that should be done is to pool these changes here).
Since I can't open issues there I'm not sure what to do.
In any case here are somethings I has issues with on @zoomsphere's version:

  1. When using @LocalStorage and @SessionStorage attributes inside a component the value is being cleared every time the component is created - which means it can't be used inside a component - which is the main example here (I think).
  2. I'm using this with angular material with a dropdown list. since the object that has the @LocalStorage attribute has a save method it can't be used to compare it to other values in a list. my code is something similar to this:
@LocalStorage()
public current: someType = { a: "a", b: "b" };
...
var list = [{a: "a", b: "b"}, {a: "1", b: "2" }];
this.current = list[0];
// here is where I spotted a problem I think, if you have an initial pluker I can probably give a better example...
this.current === list[0]; // this is false I think...

@marcj
Copy link
Owner

marcj commented May 19, 2017

Heyho guys! Awesome that you guys want to keep that library alive :) Unfortunately, as you saw I didn't have much time to maintain this beauty. I'd prefer to create a new organisation and invite everyone who wants to work on it actively. Normally, in a transfer of a open-source library that is a bit used, you just don't move it to a stranger (because it can harm pretty much everyone that uses this library when malicious code is introduced by a guy you don't know.) That's actually only about trust, so what happens usually is that guys are invited to work on it being a "official" contributor. After a while you see pretty quickly which guys take it seriously. Those guys become then usually the full owner of the repository (because they built trust). So, please let me create in the next days a organisation (any ideas?), let me move the npm package and prepare some stuff, so I can invite you.

@DanielKucal
Copy link
Author

@HarelM, thanks for your feedback! I took a look at your issues:

  1. We also do it and haven't noticed this bug. Are you sure your data is not being changed inside constructor() or ngOnInit()?
  2. That's a good point. In general it's not the best idea to mutate objects like we do it now, but it significantly simplifies the code... I think I'll make it configurable in the upcoming version. For now you can use a workaround setting current.save = undefined; before comparisons.

@HarelM
Copy link

HarelM commented May 20, 2017

Can't argue with what you said. How about using the organization and the library name the same?
I would however consider changing the library name since "2" is not a good idea IMHO.
How about:
angular.localstorage/angular.localstorage

  • angular_storage
  • angular.storage
  • angularstorage
  • @angular-storage
  • angular-storage-proxy
  • angular-storage-handler

I'll let you guys decide...
Good luck!

@DanielKucal
Copy link
Author

DanielKucal commented May 22, 2017

@marcj, thanks for your response on the topic. I thought about it and unfortunately, it sounds a bit confusing for me... Your proposition looks like a quite overkill for this young project. My questions:

  1. If you "create a new organisation and invite everyone who wants to work on it actively", I guess invited people won't have write access to the main branch, so someone still will need to check pull requests. Do you have a time for it?
  2. Second thing, the npm package... As @HarelM mentioned, current name is no longer suitable, since Angular is already on v4 and other possible names (with ngx- prefix) are taken - what will be the new name? Who will be allowed to publish new packages?
  3. How many times did it happen in the history that thriving company tries to inject malicious code to open source project?

I suppose it will take another weeks to organize everything, can't you just transfer the project?

@HarelM, I've localized the problem you mentioned (it also occurs in this version) and already prepared a solution - a bit tricky, but probably only one possible. It will be available in ngx-store today or tomorrow.

@marcj
Copy link
Owner

marcj commented May 22, 2017

Well, back in the days where v2 was newest and nobody talked about 3, "angular2-localstorage" was a decent idea. :D However, I agree that renaming it makes totally sense now.

@DanielKucal, it's never overkill to create an organisation for a project with multiple devs working on it. To your questions:

  1. No, member have write access. No need for me to maintain anything. Just keep an eye on it for few weeks. That's a process you see often when the owner/lead-dev changes in open-source projects.
  2. See above. I personally like angular.storage. what will be the new name? I have no strong opinion on that. angular in the name as prefix makes sense tho, since not all packages in npm are made for angular and it's better for google. It's up to you.
    Who will be allowed to publish new packages? whoever is in the org? I actually don't care.
    I would flag the old package as deprecated once the new is published.
  3. I see where you're coming from, but blindly trusting people isn't my thing. It's a normal process you see often when projects get a new owner and are moved.

So that's the thing I can do for you guys. Creating an org, move this repo in it, giving you full access, and remove me sooner or later when everything is working great. Since you need to create a new npm package (because of the name change) I'm really only here to give the repository in that organisation trust by having some stars and watchers right at the beginning and keep an eye on that "old" current package in npm, not because I provide anything else. Also I'll switch the npm package's git url (so we end up having 2 npm packages pointing to the same github repo) to the new repo once it's published.

@HarelM
Copy link

HarelM commented May 22, 2017

@DanielKucal - which problem are you referring to?
Just nitpicking but you should update the documentation in your repo to use ngx-store and not the git+ address.

@DanielKucal
Copy link
Author

DanielKucal commented May 25, 2017

In general I think it's a good idea. Although, it could be small inconvenience for me to devote my private time if it won't be under our organization... So I have my fingers crossed for open source community to help me develop this great project further.
Btw @HarelM, I've released new version including fix for your first issue. The second one will can be solved by setting mutateObjects to false in config in next release.

And about the name I'd rename it to ngx-store, this is a short name I've already took up. I've read somewhere that ngx- is a prefix recommended by Angular dev team, unfortunately I can't find any official source about it now.

@DanielKucal
Copy link
Author

DanielKucal commented May 26, 2017

v0.6.0 has been released! It contains important bug fixes, more configuration options, more how-tos and other cool things and... it's totally backward-compatible 💃
Just check its README 📦 for details!
I've opened the issues section, so please report everything on zoomsphere/ngx-store, thanks!

@DanielKucal
Copy link
Author

Refresh ♻️

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

No branches or pull requests

5 participants