Skip to content
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .editorconfig
Original file line number Diff line number Diff line change
Expand Up @@ -7,3 +7,6 @@ indent_size = 2
indent_style = space
insert_final_newline = true
trim_trailing_whitespace = true

[justfile]
indent_size = 4
10 changes: 6 additions & 4 deletions .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -242,17 +242,19 @@ module.exports = {
parserOptions: {
ecmaVersion: 2018,
},
plugins: ['prettier', 'import'],
extends: ['plugin:prettier/recommended'],
plugins: ['import'],
// disable formatting rules - prettier will handle this itself
extends: ['prettier'],
overrides: [
{
files: ['**/*.ts'],
plugins: ['@typescript-eslint', 'prettier'],
plugins: ['@typescript-eslint'],
extends: [
'eslint:recommended',
'plugin:@typescript-eslint/eslint-recommended',
'plugin:@typescript-eslint/recommended',
'plugin:prettier/recommended',
// disable formatting rules - prettier will handle this itself
'prettier',
],
rules: {
'@typescript-eslint/no-use-before-define': 0,
Expand Down
45 changes: 23 additions & 22 deletions .github/workflows/main.yml
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,13 @@ on:
- sdk-release/**
- feature/**


jobs:
build:
name: Build
runs-on: ubuntu-latest

steps:
- uses: extractions/setup-just@v2
- uses: actions/checkout@v2

- name: Setup node
Expand All @@ -41,30 +41,35 @@ jobs:
restore-keys: |
${{ runner.os }}-yarn-

- name: Build Typescript
run: yarn && yarn build
- name: Compile JS -> TS
run: just install build

- name: Verify Linting
run: just lint-check

- name: Lint
run: yarn lint
- name: Verify Formatting
run: just format-check

test:
name: Test (${{ matrix.node }})
needs: [build]
strategy:
fail-fast: false
matrix:
os:
- "ubuntu-latest"
- 'ubuntu-latest'
node:
# should include even numbers >= 12
# see: https://nodejs.org/en/about/previous-releases
- "22"
- "20"
- "18"
- "16"
- "14"
- "12"
- '22'
- '20'
- '18'
- '16'
- '14'
- '12'
runs-on: ${{ matrix.os }}
steps:
- uses: extractions/setup-just@v2
- uses: actions/checkout@v2

- name: Setup node
Expand All @@ -75,6 +80,7 @@ jobs:
- name: Print Node.js version
run: node -v

# used for one of the integration tests
- name: Setup Deno
uses: denoland/setup-deno@v1
with:
Expand All @@ -91,22 +97,15 @@ jobs:
id: yarn-cache
with:
path: ${{ steps.yarn-cache-dir-path.outputs.dir }}
key: ${{ runner.os }}-yarn-${{ hashFiles('**/yarn.lock') }}
# searching very deep deps can time out, so only cache on the root yarn.lock
key: ${{ runner.os }}-yarn-${{ hashFiles('yarn.lock') }}
restore-keys: |
${{ runner.os }}-yarn-

- uses: stripe/openapi/actions/stripe-mock@master

- name: Test
run: make ci-test

- name: Coveralls
run: yarn report && yarn coveralls
if: env.COVERALLS_REPO_TOKEN && matrix.node == '18'
env:
GITHUB_TOKEN: ${{ secrets.github_token }}
COVERALLS_REPO_TOKEN: ${{ secrets.COVERALLS_REPO_TOKEN }}
COVERALLS_FLAG_NAME: node-${{ matrix.node }}
run: just ci-test

publish:
name: Publish
Expand All @@ -117,6 +116,8 @@ jobs:
needs: [build, test]
runs-on: ubuntu-latest
steps:
# just is called in `yarn prepack`, which is called during the `publish` operation
- uses: extractions/setup-just@v2
- uses: actions/checkout@v2
- run: sudo apt-get install -y oathtool
- name: Publish to NPM
Expand Down
2 changes: 2 additions & 0 deletions .prettierignore
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
# internal files of the nextjs example
.next
2 changes: 2 additions & 0 deletions Makefile
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
# NOTE: this file is deprecated and slated for deletion; prefer using the equivalent `just` commands.

.PHONY: codegen-format update-version test ci-test
update-version:
@echo "$(VERSION)" > VERSION
Expand Down
19 changes: 10 additions & 9 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@

[![Version](https://img.shields.io/npm/v/stripe.svg)](https://www.npmjs.org/package/stripe)
[![Build Status](https://github.com/stripe/stripe-node/actions/workflows/main.yml/badge.svg?branch=master)](https://github.com/stripe/stripe-node/actions?query=branch%3Amaster)
[![Coverage Status](https://coveralls.io/repos/github/stripe/stripe-node/badge.svg?branch=master)](https://coveralls.io/github/stripe/stripe-node?branch=master)
[![Downloads](https://img.shields.io/npm/dm/stripe.svg)](https://www.npmjs.com/package/stripe)
[![Try on RunKit](https://badge.runkitcdn.com/stripe.svg)](https://runkit.com/npm/stripe)

Expand Down Expand Up @@ -596,14 +595,14 @@ New features and bug fixes are released on the latest major version of the `stri

## Development

Run all tests:
Run all tests (installing the dependencies first, if needed)

```bash
$ yarn install
$ yarn test
$ just install
$ just test
```

If you do not have `yarn` installed, you can get it with `npm install --global yarn`.
If you do not have `yarn` installed, consult its [installation instructions](https://classic.yarnpkg.com/lang/en/docs/install/).

The tests also depends on [stripe-mock][stripe-mock], so make sure to fetch and
run it from a background terminal ([stripe-mock's README][stripe-mock-usage]
Expand All @@ -614,32 +613,34 @@ go get -u github.com/stripe/stripe-mock
stripe-mock
```

We use [just](https://github.com/casey/just) for conveniently running development tasks. You can use them directly, or copy the commands out of the `justfile`. To our help docs, run `just`.

Run a single test suite without a coverage report:

```bash
$ yarn mocha-only test/Error.spec.ts
$ just test test/Error.spec.ts
```

Run a single test (case sensitive) in watch mode:

```bash
$ yarn mocha-only test/Error.spec.ts --grep 'Populates with type' --watch
$ just test test/Error.spec.ts --grep 'StripeError' --watch
```

If you wish, you may run tests using your Stripe _Test_ API key by setting the
environment variable `STRIPE_TEST_API_KEY` before running the tests:

```bash
$ export STRIPE_TEST_API_KEY='sk_test....'
$ yarn test
$ just test
```

Run prettier:

Add an [editor integration](https://prettier.io/docs/en/editors.html) or:

```bash
$ yarn fix
$ just format
```

[api-keys]: https://dashboard.stripe.com/account/apikeys
Expand Down
79 changes: 79 additions & 0 deletions justfile
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
set quiet := true

import? '../sdk-codegen/justfile'

# make locally installed binaries available throughout the tree without a longer specifier
# this is useful in this file, but also depended on by webhook tests that expect to be able to call `eslint` and (I think) don't set it up correctly themselves.
export PATH := `pwd` + "/node_modules/.bin:" + env('PATH')

_default:
just --list --unsorted

# this uses positional-args so that mixed quoted and unquoted arguments
# (like filtering for a certain test) work the way we expect
# ⭐ run unit tests
[positional-arguments]
test *args: build
mocha "$@"
Copy link
Member Author

Choose a reason for hiding this comment

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

This was surprisingly tricky get right! By default, just groups all the args you pass to *args, so it wasn't correctly preserving -g "some thing". This works as expected for mixed content

Copy link
Contributor

@jar-stripe jar-stripe Jan 16, 2025

Choose a reason for hiding this comment

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

why not use *args? or are the args represented in both *args and $@? and if thats true, what happens if test is called from another just rule?

Copy link
Member Author

Choose a reason for hiding this comment

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

Because of the way just expands input args. It treats them as one big group for quoting purposes, unless told otherwise. It becomes an issue when some of your args are quoted with spaces and some aren't. You can't quote the whole thing, but you also can't leave them unquoted. This is important when wanting to call mocha someFile.js -g "match this name". If the args aren't grouped correctly when passing through just, you call -g match and then this and name are extra args.

I did some testing with the following:

[positional-arguments]
arg-test *args:
    bash -c 'while (( "$#" )); do echo - $1; shift; done' -- "{{ args }}"

Which should print each arg (respecting spaces) on their own line.

"{{ args }}":

% .j arg-test a b "c d" e
- a b c d e

{{ args }}:

- a
- b
- c
- d
- e

"$@":

% .j arg-test a b "c d" e
- a
- b
- c d
- e

I mostly pulled from the docs: https://github.com/casey/just?tab=readme-ov-file#positional-arguments


# try to compile the example TS file to make sure exports work
types-test: build
tsc --build types/test

# run full integration tests by installing a bunch of packages and starting servers (slow)
integrations-test: build
RUN_INTEGRATION_TESTS=1 mocha test/Integration.spec.ts

# run the full test suite; you probably want `test`
ci-test: install test types-test integrations-test

_build mode packageType:
mkdir -p {{ mode }}
tsc -p tsconfig.{{ mode }}.json
echo '{"type":"{{ packageType }}"}' > {{ mode }}/package.json

[private]
build-esm: (_build "esm" "module")

[private]
build-cjs: (_build "cjs" "commonjs")

# generate CJS and ESM versions of the package; mostly used as a pre-req for other steps
build: build-esm build-cjs

# ⭐ run style checks, fixing issues if possible
lint: (lint-check "--fix")

# run style checks without changing anything
lint-check *args:
eslint --ext .js,.ts . {{ args }}

# reinstall dependencies, if needed
install:
yarn {{ if is_dependency() == "true" { "--silent" } else { "" } }}

[no-exit-message]
[private]
prettier *args:
# all the project-relevant JS code
prettier "{src,examples,scripts,test,types}/**/*.{ts,js}" {{ args }}

# `format` needs to install since other scripts run it cold
# ⭐ format all files
format: install (prettier "--write --loglevel silent")
# ensure other files reflect the version in package.json
./scripts/updateAPIVersion.js

# verify formatting of files (without changes)
format-check: (prettier "--check")

# called by tooling
[private]
update-version version:
echo "{{ version }}" > VERSION
perl -pi -e 's|"version": "[.\-\d\w]+"|"version": "{{ version }}"|' package.json
perl -pi -e "s|Stripe.PACKAGE_VERSION = '[.\-\d\w]+'|Stripe.PACKAGE_VERSION = '{{ version }}'|" src/stripe.core.ts

# remove build artifacts
clean:
rm -rf ./node_modules/.cache ./esm ./cjs
15 changes: 1 addition & 14 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@
"@typescript-eslint/parser": "^4.33.0",
"chai": "^4.3.6",
"chai-as-promised": "~7.1.1",
"coveralls": "^3.1.1",
"eslint": "^7.32.0",
"eslint-config-prettier": "^8.5.0",
"eslint-plugin-chai-friendly": "^0.7.2",
Expand All @@ -60,19 +59,7 @@
},
"license": "MIT",
"scripts": {
"build": "yarn build-esm && yarn build-cjs",
"build-esm": "mkdir -p esm && tsc -p tsconfig.esm.json && echo '{\"type\":\"module\"}' > esm/package.json",
"build-cjs": "mkdir -p cjs && tsc -p tsconfig.cjs.json && echo '{\"type\":\"commonjs\"}' > cjs/package.json",
"clean": "rm -rf ./.nyc_output ./node_modules/.cache ./coverage ./esm ./cjs",
"prepack": "yarn install && yarn build",
"mocha": "nyc mocha",
"mocha-only": "mocha",
"test": "yarn build && yarn test-typescript && yarn mocha",
"test-typescript": "tsc --build types/test",
"lint": "eslint --ext .js,.jsx,.ts .",
"fix": "yarn lint --fix && ./scripts/updateAPIVersion.js",
"report": "nyc -r text -r lcov report",
"coveralls": "cat coverage/lcov.info | ./node_modules/coveralls/bin/coveralls.js"
"prepack": "just install && just build"
},
"exports": {
"types": "./types/index.d.ts",
Expand Down
7 changes: 6 additions & 1 deletion scripts/updateAPIVersion.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,10 @@
#!/usr/bin/env node

/**
* Reads the current API version from src/apiVersion.ts and updates all
* references to it in the types/ directory.
*/

/* eslint-disable no-sync,no-nested-ternary */
const fs = require('fs');
const path = require('path');
Expand All @@ -12,7 +17,7 @@ const API_VERSION = '2[0-9][2-9][0-9]-[0-9]{2}-[0-9]{2}.[a-z]+';

const main = () => {
const matches = [
...read('src/apiVersion.ts').matchAll(/ApiVersion . '([^']*)'/g),
...read('src/apiVersion.ts').matchAll(/ApiVersion = '([^']*)'/g),
];
if (matches.length !== 1) {
throw new Error(
Expand Down
9 changes: 9 additions & 0 deletions test/Integration.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,15 @@ import {FAKE_API_KEY} from './testUtils.js';
const nodeVersion = parseInt(process.versions.node.split('.')[0], 10);

describe('Integration test', function() {
// these tests are expensive and start processes they don't clean up
Copy link
Member Author

Choose a reason for hiding this comment

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

This is why your CPU goes to 100% when running the test suite locally!

Copy link
Contributor

Choose a reason for hiding this comment

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

is there a fix we can make here? or is it more complicated than can be fixed as part of this PR?

Copy link
Member Author

Choose a reason for hiding this comment

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

It's fixable, but IMO out of scope of this PR. I didn't want to get too distracted. Plus we don't care if we're leaking on CI, since it all gets shut down when the job completes

// so, skip them in the regular test suite (which we run locally) and run them via `just test-integrations`
// (which is also called in CI)
before(function() {
if (process.env.RUN_INTEGRATION_TESTS !== '1') {
this.skip();
}
});

this.timeout(50000);
const testExec = (cmd: string): Promise<void> => {
const child = childProcess.exec(cmd);
Expand Down
Loading
Loading