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

how is this related with angular 2? #8

Closed
samvloeberghs opened this issue Apr 15, 2016 · 8 comments
Closed

how is this related with angular 2? #8

samvloeberghs opened this issue Apr 15, 2016 · 8 comments
Labels

Comments

@samvloeberghs
Copy link

Hi,

how is this related with angular2? It's just a Cookie wrapper implementation..
(just a question)

grtz

@Bigous
Copy link
Member

Bigous commented Apr 18, 2016

That's it, a Cookie wrapper in typescript, respecting the new angular 2 specifications... So you can, in an easy way, npm install, import and use it just like all other angular2 libraries.

@Bigous Bigous closed this as completed Apr 18, 2016
@naeramarth7
Copy link

So you can, in an easy way, npm install, import and use it just like all other angular2 node libraries.

I totally agree with @samvloeberghs. As he stated, the name is confusing as it is not in any way related to angular2 but rather to any bundled front end app - even without a framework.

Providing an observable implementing angular2's NgZone or ChangeDetectorRef would make it deserve the ng2 prefix. Just writing something in TypeScript does not make something exclusively usable for angular2, folks!

@samvloeberghs
Copy link
Author

finally :)

@Bigous
Copy link
Member

Bigous commented Feb 20, 2017

The intent of this library is to be used only with angular2, but it sure can be used without it. ;)

@naeramarth7
Copy link

"ng2" Prefix

The intent of this library is to be used only with angular2

Is that so? Please explain, what makes this exclusive to Angular2?

For example... Is there any reason for using this over js-cookie/js-cookie with @types/js-cookie - a widespread library including type definitions which actually works as described?

Issues with npm package

When installing ng2-cookies via npm (node@6.9.4), the following files are included:

src/services:
total 12K
-rw-r--r-- 1 user group 1.6K May 17  2016 cookie.d.ts
-rw-r--r-- 1 user group 3.2K May 17  2016 cookie.js
-rw-r--r-- 1 user group 3.0K May 17  2016 cookie.js.map

This is what ng2-cookies/ng2-cookies.d.ts actually exports.

The latest README.md tells us to import ng2-cookies/ng2-cookies, which will import this outdated files. Looks like it does this for months already. Therefore there exist issues as:

#29 Cookie not null

Outdated implementation in ng2-cookies/ng2-cookies.

#30 Cookie.deleteAll() does not work

Outdated implementation in ng2-cookies/ng2-cookies.

#33 Cookie.check is not defined

Does actually not even exist in ng2-cookies/ng2-cookies.

... just to name a few you closed "due to the lack of activity".

Though, the real issue is a) the maintainer troubles publishing to npmjs.org and b) the documentation is outdated. Funny thing, in your comments (e.g. for 30 and 33) you import ng2-cookies which obviously works, since it's a totally different import.

Note: Simply write a test instead of repeatedly pasting the same code into comments and ask people to copy and test that stuff.

"ng2" again

In 30 someone already mentioned that the import taken from the documentation is wrong. Instead of fixing it, the reaction was:

I'm not sure, but angular 2 recommends the way it's in the example... can be...

You know, that this still has nothing - seriously: nothing - to do with Angular2, but rather with CommonJS modules and the way you import (instead of require) them using TypeScript (see TypeScript modules and TypeScript module resolution ?
As you stated, you obviously don't.

Angluar2 exports something out of @angular/core, because there actually is something inside of the @angular/core folder that can be imported - this is by intention and not just a coincidence.

Conclusion

In my honest opinion, this is nothing but a clickbait for Angular2 beginners. But even those should ask, what this actually provides you... No Decorator, no Service, not even an Observable (though, that's not even Angular2 but rather RxJS).

Sadly, same goes for other repositories as Bigous/ng2-highcharts which do nothing but to execute something on globally available variables / frameworks - which again has nothing to do with Angular2 per se.

@Bigous
Copy link
Member

Bigous commented Feb 21, 2017

Ok, big post, and I'm a busy person so, short answers.

Fist, thanks for helping me note that ReadMe was pointing to old way of doing things (his library was written when angular2 was beta-1) - it was a lack of update, but you could help people from all the issues you pointed sending a pull request instead of staying in the shadow pointing fingers.

About the "ng2 problem", you use the library you want... you don't have to use this library because it starts with ng2 - so, it's a pointless discussion.

@sebastianhaas
Copy link

Don't think this is pointless, you are misleading people (possibly "busy persons" like you) that don't (have the time to) look under the hood of this project.

Oh and btw. what you are doing here is not working with Angular 2 AOT compiler. So I tend to go with @samvloeberghs and @naeramarth7, you should either try and rework this library towards a more Angular 2 specific solution (like angular2-cookie, but maybe actually working) or just get rid of the ng2 prefix.

And one more personal note, if some random guy on GH is trying to give you elaborately, constructive feedback, I don't feel like

Ok, big post, and I'm a busy person so, short answers.

is an appropriate answer.

@Bigous Bigous mentioned this issue Mar 1, 2017
@Bigous
Copy link
Member

Bigous commented Mar 1, 2017

I don't know why would I mislead someone if the library does what it was created to do.

I tried with the minimum example at wiki and ng build --prod --aot worked normally to me, if you are having any problem with AOT feel free to open an issue. But I've rewritten the build with ngc instead of tsc and it is under branch fix-34 (you can see the issue #34 for more details). It was not merged to master because I could not see a real gain, but it's there...

And about the personal note, I understood. It was more a though not an answer, but the answer comes after that frase too. I'll try to stick to the answer only from now on, because I think you're right.

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

No branches or pull requests

5 participants