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

chore: Drop CI support for Node 16 (EOL), use pnpm 9 internally #1723

Merged
merged 12 commits into from
Feb 10, 2025

Conversation

robbie-c
Copy link
Member

@robbie-c robbie-c commented Feb 10, 2025

Changes

We use pnpm 9 in posthog/posthog, so use it here for consistency.

This requires dropping CI support for Node 16, but this is EOL, and shouldn't be used regardless.

See:

Checklist

  • Tests for new code (see advice on the tests we use)
  • Accounted for the impact of any changes across different browsers
  • Accounted for backwards compatibility of any changes (no breaking changes in posthog-js!)

Copy link

vercel bot commented Feb 10, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
posthog-js ✅ Ready (Inspect) Visit Preview Feb 10, 2025 0:46am

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

PR Summary

This PR standardizes the pnpm package manager version to 9.x.x across the posthog-js repository to match the main PostHog repository's configuration.

  • Added packageManager field in package.json files to enforce pnpm@9.15.4
  • Updated pnpm version from 8.x.x to 9.x.x across all GitHub workflow files
  • Upgraded posthog-js dependency in playground/nextjs from 1.200.1 to 1.208.1
  • Inconsistent usage of actions/checkout versions (v2 vs v4) in CD workflow needs standardization
  • Mixed usage of npm/pnpm commands in react/package.json scripts should be unified

12 file(s) reviewed, 4 comment(s)
Edit PR Review Bot Settings | Greptile

@@ -30,7 +30,7 @@ jobs:
- uses: actions/checkout@v4
- uses: pnpm/action-setup@v4
with:
version: 8.x.x
version: 9.x.x
- uses: actions/setup-node@v4
with:
node-version: '18'
Copy link
Contributor

Choose a reason for hiding this comment

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

style: Missing cache: 'pnpm' configuration for Node setup in integration job, while it's present in other jobs

Suggested change
node-version: '18'
node-version: '18'
cache: 'pnpm'

@@ -38,7 +38,7 @@ jobs:
- uses: actions/checkout@v4
- uses: pnpm/action-setup@v4
with:
version: 8.x.x
version: 9.x.x
Copy link
Contributor

Choose a reason for hiding this comment

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

style: Consider pinning to a specific 9.x version rather than using 9.x.x to ensure consistent builds

@@ -21,7 +22,7 @@
"eslint": "^8.57.1",
Copy link
Contributor

Choose a reason for hiding this comment

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

style: eslint should typically be in devDependencies since it's a development tool, not a runtime dependency

Suggested change
"eslint": "^8.57.1",
"eslint": "^8.57.1"

@@ -10,6 +10,7 @@
},
"author": "hey@posthog.com",
"license": "MIT",
"packageManager": "pnpm@9.15.4",
"scripts": {
"prebuild": "npm run clean",
Copy link
Contributor

Choose a reason for hiding this comment

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

style: Should use pnpm clean instead of npm run clean for consistency with other scripts

Suggested change
"prebuild": "npm run clean",
"prebuild": "pnpm clean",

Copy link

github-actions bot commented Feb 10, 2025

Size Change: 0 B

Total Size: 3.3 MB

ℹ️ View Unchanged
Filename Size
dist/all-external-dependencies.js 215 kB
dist/array.full.es5.js 267 kB
dist/array.full.js 370 kB
dist/array.full.no-external.js 369 kB
dist/array.js 183 kB
dist/array.no-external.js 181 kB
dist/customizations.full.js 13.8 kB
dist/dead-clicks-autocapture.js 14.5 kB
dist/exception-autocapture.js 9.48 kB
dist/external-scripts-loader.js 2.64 kB
dist/main.js 183 kB
dist/module.full.js 370 kB
dist/module.full.no-external.js 369 kB
dist/module.js 183 kB
dist/module.no-external.js 181 kB
dist/recorder-v2.js 115 kB
dist/recorder.js 115 kB
dist/surveys-preview.js 69.4 kB
dist/surveys.js 72.4 kB
dist/tracing-headers.js 1.76 kB
dist/web-vitals.js 10.4 kB

compressed-size-action

@robbie-c robbie-c changed the title chore: Bump package.json to match main posthog repo chore: Drop CI support for Node 16 (EOL), use pnpm 9 internally Feb 10, 2025
Copy link
Member

@pauldambra pauldambra left a comment

Choose a reason for hiding this comment

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

README mentions pnpm 8 as well...

16 went end of line Sept 2023... saying we drop support ~ 1 year later feels safe

🤞

@robbie-c robbie-c added the bump minor Bump minor version when this PR gets merged label Feb 10, 2025
@robbie-c robbie-c merged commit 4f7300a into main Feb 10, 2025
28 checks passed
@robbie-c robbie-c deleted the chore/bump-pnpm-to-match-main-repo branch February 10, 2025 13:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bump minor Bump minor version when this PR gets merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants