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

Added typescript webhook example #749

Closed
wants to merge 1 commit into from

Conversation

paulasjes-stripe
Copy link
Contributor

Copy link
Contributor

@rattrayalex-stripe rattrayalex-stripe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice!

few questions, and we should maybe hold off on merging till TS lands so we can remove line 5 of the tsconfig?

res: express.Response,
next: express.NextFunction
): void => {
if (req.originalUrl === '/webhooks') {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this our best-practice? Does line 55 not work here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Line 55 does work, but I've seen people get tripped up by how express' middleware works. Having this block here will hopefully nip that confusion in the bud (and makes it so all other routes use the json body parser).


/* Strict Type-Checking Options */
"strict": true /* Enable all strict type-checking options. */,
"esModuleInterop": true /* Enables emit interoperability between CommonJS and ES Modules via creation of namespace objects for all imports. Implies 'allowSyntheticDefaultImports'. */,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this is needed any longer

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's needed for line 5 of express-ts.ts where we import dotenv.

"compilerOptions": {
"target": "es5" /* Specify ECMAScript target version: 'ES3' (default), 'ES5', 'ES2015', 'ES2016', 'ES2017', 'ES2018', 'ES2019' or 'ESNEXT'. */,
"module": "commonjs" /* Specify module code generation: 'none', 'commonjs', 'amd', 'system', 'umd', 'es2015', or 'ESNext'. */,
"types": ["../../types/2019-12-03"],
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This shouldn't be needed once it lands, but I guess we can keep it in till then?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I wasn't sure if this is necessary or not, happy to remove once v8 lands.

'/webhooks',
bodyParser.raw({type: 'application/json'}),
(req: express.Request, res: express.Response): express.Response | void => {
const sig: string = req.headers['stripe-signature'] as string;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is fixed; you no longer need to cast!

@remi-stripe remi-stripe deleted the paulasjes/typescript-example branch February 12, 2020 16:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants