upgrade rollup out of the stone age #480
Conversation
madhav-stripe
changed the title
| @@ -11,7 +11,7 @@ jobs: | |||
| - uses: actions/checkout@v2 | |||
| - uses: actions/setup-node@v1 | |||
| with: | |||
| node-version: 14.x | |||
| node-version: 20.x | |||
There was a problem hiding this comment.
modern versions of rollup do not support node < 18. Since Node.js v20 is available and LTS, we are upgrading the action runner to use Node v20
| @@ -3,8 +3,8 @@ | |||
| "version": "2.5.1", | |||
| "description": "React components for Stripe.js and Stripe Elements", | |||
| "main": "dist/react-stripe.js", | |||
| "module": "dist/react-stripe.esm.js", | |||
There was a problem hiding this comment.
I ran npx publint and it gave the suggestion to rename the exports here from js to mjs.
When consuming this package there might be some ambiguity onto wether this file is ESM or Common JS (because it ends with .js). Changing the file extension to .mjs explicitly calls out that this files uses ESM
| @@ -14,7 +14,7 @@ | |||
| "lint": "eslint --max-warnings=0 '{src,examples}/**/*.{ts,tsx,js}'", | |||
| "lint:prettier": "prettier './**/*.js' './**/*.ts' './**/*.tsx' './**/*.css' './**/*.md' --list-different", | |||
| "typecheck": "tsc", | |||
| "build": "yarn run clean && yarn run rollup -c && yarn checkimport", | |||
| "build": "yarn run clean && yarn run rollup -c --bundleConfigAsCjs && yarn checkimport", | |||
There was a problem hiding this comment.
the model version of rollup expects the rollup config to be in ESM (if using the .js) syntax. To not have a larger diff than needed - passing this flag loads a .js config as CommonJS
package.json
Outdated
| @@ -54,13 +54,18 @@ | |||
| } | |||
| }, | |||
| "dependencies": { | |||
| "@rollup/plugin-babel": "^6.0.4", | |||
There was a problem hiding this comment.
the previous versions of these plugins were named rollup-plug-xxxx. Rollup has since upgraded and changed the name of these plugins to the @rollup/plugin-xxx naming convention.
| @@ -71,7 +76,6 @@ | |||
| "@types/react-dom": "^18.0.0", | |||
| "@typescript-eslint/eslint-plugin": "^2.18.0", | |||
| "@typescript-eslint/parser": "^2.18.0", | |||
| "@wessberg/rollup-plugin-ts": "^1.2.15", | |||
There was a problem hiding this comment.
The author of this plugin has published new versions of this plugin as rollup-plugin-ts
| "ts-jest": "^25.1.0", | ||
| "ts-loader": "^6.2.1", | ||
| "typescript": "^3.7.5" | ||
| "typescript": "^4.1.2" |
There was a problem hiding this comment.
upgrading typescript here to match the typescript version in stripe-js
| @@ -61,6 +61,11 @@ | |||
| "@babel/core": "^7.7.2", | |||
| "@babel/preset-env": "^7.7.1", | |||
| "@babel/preset-react": "^7.7.0", | |||
| "@rollup/plugin-babel": "^6.0.4", | |||
There was a problem hiding this comment.
the older versions of rollup plugins were in the format rollup-plugin-xxxxx, it has now changed to `@rollup/plugin-xxxxx
Summary & motivation
In this PR we are upgrading rollup and our build configurations to the most recent / modern versions of the libraries. I will do my best to explain what has been upgraded. I've added comments inline to where the changes have been made
API review
Copy this template or link to an API review issue.
Testing & documentation