cljsjs / packages

DEPRECATED: Javascript libraries packaged up with Google Closure externs
http://cljsjs.github.io
787 stars 816 forks source link

stripe js is version 3 now #1213

Open thedavidmeister opened 7 years ago

thedavidmeister commented 7 years ago

I had a bit of a look at this.

So, one thing that hasn't changed from v2 to v3 is that Stripe recommend that the JS is always loaded from https://js.stripe.com - https://stripe.com/docs/stripe.js

But the API methods look pretty different in v3.

Now there's a global Stripe object that takes the public key in the constructor.

They also have a new "elements" API that wasn't in the v2 extern. Basically it lets you call Stripe.elements() to get an "elements object" then call elements.create() to get an "element object" and then element.mount(my-dom-element) to actually get something into the dom.

So it's a bit like this (in JS, ignoring various optional arguments):

var stripe = Stripe('my-public-key');
var elements = stripe.elements();
var element = elements.create('card');
var domElement = document.getElementById('#foo');
element.mount(domElement);

Unfortunately this is getting to the limits of my extern-fu so I'll need a bit of help with structuring an externs file to cover chaining methods that return new objects like this Stripe -> elements -> element.

Deraen commented 7 years ago

Chaining methods doesn't really matter at all in externs. The only thing that matter is that extern defines the used name (function/method) somewhere, it doesn't even matter if the name is defined as global function or under different object.

var Stripe = {};
Stripe.elements = function() {};
Stripe.create = function() {};
Stripe.mount = function() {};

;;; equivalent
Object.Stripe = {};
Object.elements = function() {};
Object.create = function() {};
Object.mount = function() {}

;;; equivalent
function Stripe () {};
function elements() {};
function create() {};
function mount() {}
thedavidmeister commented 7 years ago

@Deraen oh wow, that's interesting.

So why do we jump through all the hoops of making this crazy pseudo-JS stuff in the first place?

Why not simply provide a list of "reserved words" like ["Stripe" "elements" "create" "mount"] in a .edn somewhere and leave it at that?

Deraen commented 7 years ago

Externs are for Closure and it has always used these JS files. It would be possible to write tooling to write externs from edn data but no one has done so, probably as it is not that much easier than extern files. Externs inference will eventually provide easier solution which generates the extern files based on JS calls in Cljs code: https://clojurescript.org/guides/externs#externs-inference

Deraen commented 7 years ago

Also, there are SOME cases where providing JSDoc type annotations in extern files could result in better Closure compiled code, e.g. /** @const {boolean} */ var CONSTANT_VALUE = false; and using this constant from Cljs code (when js/CONSTANT_VALUE ...), Closure MIGHT be able to optimize code to e.g. remove if-branches which will never be used. (Not sure if this example really works, but AFAIK Closure can use externs type info.)

thedavidmeister commented 7 years ago

hmmmm, not that i have the time to pursue this atm, but i think a relatively flat EDN file would be much easier to understand than the current externs files - and the annotations could be tackled with metadata or similar.

sherbondy commented 6 years ago

Hi folks,

I'd like to get this squared away!

I made an attempt at generating the externs for the Stripe library using an unconventional method, since the externs generator is not up to the task because of the initialization pattern used for the stripe js library as you all detailed.

Anyway, I discovered in the TypeScript community's analogous project, DefinitelyTyped, somebody graciously submitted a TypeScript Declaration file for the Stripe v3 library:

https://github.com/DefinitelyTyped/DefinitelyTyped/commits/master/types/stripe-v3/index.d.ts

I subsequently passed this through tsickle, Google's TypeScript => Google Closure conversion tool. And it spit out something that looks reasonable:

https://github.com/sherbondy/packages/blob/3b76608fa7fd5f409dd0e186ea5046020030386d/stripe/resources/cljsjs/common/stripe.ext.js

Anyway, I've submitted a PR corresponding to that file, but I've got two open questions that I'd appreciate guidance on (also included in the PR comments):

  1. If the externs file for stripe.js that I generated seems legit / up-to-snuff since I used the unconventional route of passing a TypeScript Definitions file through tsickle to create it.

  2. Whether it is better to split the stripe externs-only package into a separate stripe_3 package since it seem that stripe_2 isn't going away anytime soon (and could hypothetically get tweaked in a way that could impact the externs?), and stripe v3 is a radical departure...