search

naustudio / vn-payments

Various VN Payment Gateways implemented for NodeJS
http://code.naustud.io/vn-payments
Apache License 2.0
134 stars 41 forks source link

Remove URL import due to URL Class is now available on the global object (node 10) #18

Closed hoantran-it closed 5 years ago

hoantran-it commented 5 years ago

The PR is used to solve this issue https://github.com/naustudio/node-vn-payments/issues/17

Unit test result

Screen Shot 2019-07-04 at 5 29 09 PM
codecov[bot] commented 5 years ago

Codecov Report

Merging #18 into master will not change coverage. The diff coverage is n/a.

Impacted file tree graph

@@          Coverage Diff          @@
##           master    #18   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files           7      7           
  Lines         290    290           
  Branches       40     40           
=====================================
  Hits          290    290
Impacted Files Coverage Δ
src/onepay/OnePay.js 100% <ø> (ø) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 3dd7090...2039c32. Read the comment docs.

trongthanh commented 5 years ago

@tttt-conan please help review the impact. We need to update other classes.

@hoantran-it What if we want to support Node.js 8 (it's still in maintenance)?

tttt-conan commented 5 years ago

I can't reproduce the issue with both node v10.15.3 and v10.16.0. But the URL class still must be accessed via require('url').URL in older node version like version 8.

hoantran-it commented 5 years ago

Ok, I think we can by pass this issue now, just keep awareness for future maintenance. Thank for your effort!