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.

Sign up for GitHub

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

upgrade rollup out of the stone age #480

Merged
merged 3 commits into from
Mar 8, 2024
Merged

upgrade rollup out of the stone age #480

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f557f2e
upgrade rollup
madhav-stripe Mar 5, 2024
421e48c
upgrade nodejs action version
madhav-stripe Mar 5, 2024
698cf38
move deps to dev deps
madhav-stripe Mar 6, 2024
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
2 changes: 1 addition & 1 deletion .github/workflows/build.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ jobs:
- uses: actions/checkout@v2
- uses: actions/setup-node@v1
with:
node-version: 14.x
node-version: 20.x
Copy link
Contributor Author

Choose a reason for hiding this comment

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

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

- run: yarn install --frozen-lockfile
- run: yarn run lint:prettier
- run: yarn run lint
Expand Down
22 changes: 11 additions & 11 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

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

"jsnext:main": "dist/react-stripe.esm.js",
"module": "dist/react-stripe.esm.mjs",
"jsnext:main": "dist/react-stripe.esm.mjs",
"browser:min": "dist/react-stripe.umd.min.js",
"browser": "dist/react-stripe.umd.js",
"types": "dist/react-stripe.d.ts",
Expand All @@ -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",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

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

"checkimport": "scripts/check-imports",
"clean": "rimraf dist",
"prettier:fix": "prettier './**/*.js' './**/*.ts' './**/*.tsx' './**/*.css' './**/*.md' --write",
Expand Down Expand Up @@ -54,13 +54,18 @@
}
},
"dependencies": {
"@rollup/plugin-babel": "^6.0.4",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

"@rollup/plugin-terser": "^0.4.4",
"prop-types": "^15.7.2"
},
"devDependencies": {
"@babel/cli": "^7.7.0",
"@babel/core": "^7.7.2",
"@babel/preset-env": "^7.7.1",
"@babel/preset-react": "^7.7.0",
"@rollup/plugin-commonjs": "^25.0.7",
"@rollup/plugin-node-resolve": "^15.2.3",
"@rollup/plugin-replace": "^5.0.5",
"@storybook/react": "^6.5.0-beta.8",
"@stripe/stripe-js": "^2.2.0",
"@testing-library/jest-dom": "^5.16.4",
Expand All @@ -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",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The author of this plugin has published new versions of this plugin as rollup-plugin-ts

"babel-eslint": "^10.0.3",
"babel-jest": "^24.9.0",
"babel-loader": "^8.0.6",
Expand All @@ -92,15 +96,11 @@
"react-dom": "18.1.0",
"react-test-renderer": "^18.0.0",
"rimraf": "^2.6.2",
"rollup": "^1.27.0",
"rollup-plugin-babel": "^4.3.3",
"rollup-plugin-commonjs": "^10.1.0",
"rollup-plugin-node-resolve": "^5.2.0",
"rollup-plugin-replace": "^2.2.0",
"rollup-plugin-terser": "^5.1.2",
"rollup": "^4.12.0",
"rollup-plugin-ts": "^3.4.5",
"ts-jest": "^25.1.0",
"ts-loader": "^6.2.1",
"typescript": "^3.7.5"
"typescript": "^4.1.2"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

upgrading typescript here to match the typescript version in stripe-js

},
"resolutions": {
"@types/react": "18.0.5"
Expand Down
17 changes: 9 additions & 8 deletions rollup.config.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,22 +1,23 @@
import babel from 'rollup-plugin-babel';
import commonjs from 'rollup-plugin-commonjs';
import resolve from 'rollup-plugin-node-resolve';
import {terser} from 'rollup-plugin-terser';
import replace from 'rollup-plugin-replace';
import ts from '@wessberg/rollup-plugin-ts';
import {babel} from '@rollup/plugin-babel';
import commonjs from '@rollup/plugin-commonjs';
import {nodeResolve} from '@rollup/plugin-node-resolve';
import replace from '@rollup/plugin-replace';
import terser from '@rollup/plugin-terser';
import ts from 'rollup-plugin-ts';
import pkg from './package.json';

const PLUGINS = [
commonjs(),
ts(),
resolve(),
nodeResolve(),
babel({
extensions: ['.ts', '.js', '.tsx', '.jsx'],
}),
replace({
'process.env.NODE_ENV': JSON.stringify('production'),
_VERSION: JSON.stringify(pkg.version),
preventAssignment: true,
}),
commonjs(),
];

export default [
Expand Down
2 changes: 1 addition & 1 deletion scripts/check-imports
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,4 +23,4 @@ checkImport() {
}

checkImport "/dist/react-stripe.d.ts" 'import [^*{]' 'Please only use * or named imports for types' && \
checkImport "/dist/react-stripe.esm.js" 'import.*{' 'Please do not use named imports for dependencies'
checkImport "/dist/react-stripe.esm.mjs" 'import.*{' 'Please do not use named imports for dependencies'
Loading
Loading