richnologies / ngx-stripe

Angular 6+ wrapper for StripeJS
MIT License
219 stars 77 forks source link

Issue with example code. #9

Closed einewton closed 7 years ago

einewton commented 7 years ago

Hello,

When I go to the view with the this.card.mount() and go away from the view, then return to that view again I get the following error:

core.es5.js:1020 ERROR TypeError: Cannot read property 'nativeElement' of undefined
    at SafeSubscriber._next (paymentcomponent.component.ts:154)
    at SafeSubscriber.webpackJsonp.../../../../rxjs/Subscriber.js.SafeSubscriber.__tryOrSetError (Subscriber.js:247)
    at SafeSubscriber.webpackJsonp.../../../../rxjs/Subscriber.js.SafeSubscriber.next (Subscriber.js:187)
    at Subscriber.webpackJsonp.../../../../rxjs/Subscriber.js.Subscriber._next (Subscriber.js:125)
    at Subscriber.webpackJsonp.../../../../rxjs/Subscriber.js.Subscriber.next (Subscriber.js:89)
    at MapSubscriber.webpackJsonp.../../../../rxjs/operator/map.js.MapSubscriber._next (map.js:83)
    at MapSubscriber.webpackJsonp.../../../../rxjs/Subscriber.js.Subscriber.next (Subscriber.js:89)
    at FilterSubscriber.webpackJsonp.../../../../rxjs/operator/filter.js.FilterSubscriber._next (filter.js:87)
    at FilterSubscriber.webpackJsonp.../../../../rxjs/Subscriber.js.Subscriber.next (Subscriber.js:89)
    at BehaviorSubject.webpackJsonp.../../../../rxjs/BehaviorSubject.js.BehaviorSubject._subscribe (BehaviorSubject.js:28)

Line 154 is this line from your example: this.card.mount(this.cardRef.nativeElement);

The nativeElement is a property for the cardRef using the angular ElementRef and When reading about it, they say:

USE WITH CAUTION

Use this API as the last resort when direct access to DOM is needed. Use templating and data-binding provided by Angular instead. Alternatively you take a look at Renderer which provides API that can safely be used even when direct access to native elements is not supported. Relying on direct DOM access creates tight coupling between your application and rendering layers which will make it impossible to separate the two and deploy your application into a web worker.

Should you be using something else, or a better way of doing this?

When looking at that reference document that I linked above (and here) it shows a security issue, and being a payment I feel this should be top priority to find a better way than using ElementRef.. Here is the note from Angular.IO

Security Risk

Permitting direct access to the DOM can make your application more vulnerable to XSS attacks.

einewton commented 7 years ago

Ok, I fixed this by taking out the card setup code (this.stripeService.elements) from ngOnInit and putting it into ngAfterViewInit.

richnologies commented 7 years ago

About the other concerns, you can remove the ElementRef and pass the selector id to the mount call:

this.card.mount('#card-element')

So no more cardRef, no ElementRef and no nativeElement

Thanks for the fix with ngAfterViewInit. I will update the readme tomorrow.