Re-introduce Typescript changes #1551
Conversation
anniel-stripe
force-pushed
the
anniel-typescript-take2
branch
from
cec1ab3
to
b523144
Compare
Co-authored-by: pakrym-stripe <99349468+pakrym-stripe@users.noreply.github.com>
anniel-stripe
force-pushed
the
anniel-typescript-take2
branch
from
7a993ff
to
3d63ada
Compare
…to import = require for migrated files
anniel-stripe
force-pushed
the
anniel-typescript-take2
branch
from
7d78602
to
c9cd4b5
Compare
anniel-stripe
force-pushed
the
anniel-typescript-take2
branch
from
1fd23d4
to
6eb107f
Compare
anniel-stripe
force-pushed
the
anniel-typescript-take2
branch
from
6eb107f
to
15ccbca
Compare
|
LGTM. Let's talk with @xavdid-stripe tomorrow. |
There was a problem hiding this comment.
I gave the following files a fairly thorough read:
- StripeMethod
- StripeResource
- autoPagination.js
- makeRequest.js
- stripe.js
- Error.js
I spot checked a few more, but stopped when they mostly had whitespace lines removed.
Nothing jumped out as being functionally different in those files - most changes were formatting-only (or explicitly commented on as a change).
If you're looking for a more thorough comparison, one option for surfacing any non-superficial changes would be to take src/x.js and lib/x.js either:
- run them both through a formatter like
prettierand then diff that result - diff the ASTs of each, so we can better compare structural changes vs formatting ones
Happy to run with that a bit if it's something you're interested in! Or we can probably call this "good enough" 😁
| _errorHandler(req, requestRetries, callback) { | ||
| return (message, detail) => { | ||
| callback.call( | ||
| this, | ||
| new StripeConnectionError({ | ||
| message: this._generateConnectionErrorMessage(requestRetries), | ||
| detail: error, |
There was a problem hiding this comment.
Looks like this line changed. Was that intentional?
| const makeRequest = (apiVersion, headers, numRetries) => { | ||
| // timeout can be set on a per-request basis. Favor that over the global setting | ||
| const timeout = | ||
| options.settings && | ||
| options.settings.timeout && |
There was a problem hiding this comment.
this is TS being extra defensive, no problem there.
| @@ -598,11 +518,9 @@ StripeResource.prototype = { | |||
| options.headers, | |||
| options.settings | |||
| ); | |||
|
|
|||
| makeRequest(apiVersion, headers); | |||
| makeRequest(apiVersion, headers, 0); | |||
There was a problem hiding this comment.
This 0 is an addition- I remember a comment from a previous revision, but it seems to have been wiped from GH.
| @@ -150,12 +133,10 @@ const utils = (module.exports = { | |||
| if (typeof arg === 'string') { | |||
| opts.auth = args.pop(); | |||
| } else if (utils.isOptionsHash(arg)) { | |||
| const params = {...args.pop()}; | |||
|
|
|||
| const params = Object.assign({}, args.pop()); | |||
There was a problem hiding this comment.
this is functionally equivalent
| // Expose StripeResource on the instance too | ||
| this.StripeResource = Stripe.StripeResource; | ||
| } | ||
|
|
||
| Stripe.errors = require('./Error'); | ||
| Stripe.errors = _Error; |
There was a problem hiding this comment.
These are just repoints, also fine
Summary
r? @kamil-stripe
Re-introduce the changes reverted in #1547 , with additional changes to minimize differences between the original and TypeScript-compiled JavaScript code.
The first 3 commits of this PR are separated as follows:
Object.defineProperty(exports, "__esModule", { value: true });The remaining commits add test projects importing
stripewith both CommonJS- and ES-style imports, and also updates imports / exports to follow this pattern:module.exportstoexport =syntax in migrated TS files, export for instripe.tsto preserve named and default exports.const = requiretoimport = requiresyntax for modules that have been migrated to TypeScript to get type checking