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

Upgrade with-redux example to app router #49994

Merged
merged 15 commits into from
Jun 16, 2023

Conversation

dvakatsiienko
Copy link
Contributor

What?

Update with-redux example.

Why?

with-redux example have:

  • outdated packages
  • outdated approaches and relies on pages directory

Since app router is stable now and is recommended to use, I've updated this example.

How?

An update includes:

  • move example to app router
  • update package.json deps to the latest versions
  • modernize jest by switching from ts-node to @swc/jest
  • fix and overhaul tests
  • modernize redux infra
  • overhaul example source code quality

@dvakatsiienko dvakatsiienko requested review from a team, leerob and steven-tey as code owners May 18, 2023 13:22
@ijjk ijjk added the examples Issue was opened via the examples template. label May 18, 2023
@dvakatsiienko dvakatsiienko changed the title Update with-redux example to app router Upgrade with-redux example to app router May 18, 2023
@dvakatsiienko
Copy link
Contributor Author

Hi, @leerob @steven-tey

It seems that Github Actions CI got broken at Socket Security: Pull Request Alerts and it blocks CI pipeline to advance.

This happens for multiple PR's related to examples directory. I've created an issue for that.

@socket-security
Copy link

socket-security bot commented May 26, 2023

New dependency changes detected. Learn more about Socket for GitHub ↗︎


🚨 Potential security issues found in this pull request. To accept the risk, merge this PR and you will not be notified again.

Bot Commands

To ignore an alert, reply with a comment starting with @SocketSecurity ignore followed by a space separated list of package-name@version specifiers. e.g. @SocketSecurity ignore foo@1.0.0 bar@* or ignore all packages with @SocketSecurity ignore-all

  • @SocketSecurity ignore @swc/core@1.3.60
📜 Install scripts

Install scripts are run when the package is installed. The majority of malware in npm is hidden in install scripts.

Packages should not be running non-essential scripts during install and there are often solutions to problems people solve with install scripts that can be run at publish time instead.

Package Script field Source
@swc/core@1.3.60 (added) postinstall examples/with-redux/package.json via @swc/jest@0.2.26
Pull request alert summary
Issue Status
Install scripts ⚠️ 1 issue
Native code ✅ 0 issues
Bin script shell injection ✅ 0 issues
Unresolved require ✅ 0 issues
Invalid package.json ✅ 0 issues
HTTP dependency ✅ 0 issues
Git dependency ✅ 0 issues
Potential typo squat ✅ 0 issues
Known Malware ✅ 0 issues
Telemetry ✅ 0 issues
Protestware/Troll package ✅ 0 issues

📊 Modified Dependency Overview:

➕ Added Package Capability Access +/- Transitive Count Publisher
@swc/core@1.3.60 filesystem, shell, environment +10 kdy1
@types/redux-logger@3.0.9 None +0 types
waait@1.0.5 None +0 wesbos
react-redux@8.0.5 None +0 acemarke
@swc/jest@0.2.26 filesystem +12 kdy1
jest-watch-typeahead@2.2.2 None +4 simenb

🚮 Removed packages: @testing-library/user-event@13.5.0, ts-jest@27.1.5

@dvakatsiienko dvakatsiienko force-pushed the update-with-redux-example branch 2 times, most recently from b34649c to 8510db0 Compare May 28, 2023 09:00
Copy link
Member

@leerob leerob left a comment

Choose a reason for hiding this comment

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

It would be great to simplify this example. Remote saas, remove testing, and focus specifically on Next.js + Redux only in the most minimal, yet complete, example.

@dvakatsiienko
Copy link
Contributor Author

Sounds like a good idea, I'll simplify it.

Verified

This commit was signed with the committer’s verified signature.
dvakatsiienko Dima Vakatsiienko

Verified

This commit was signed with the committer’s verified signature.
dvakatsiienko Dima Vakatsiienko

Verified

This commit was signed with the committer’s verified signature.
dvakatsiienko Dima Vakatsiienko

Verified

This commit was signed with the committer’s verified signature.
dvakatsiienko Dima Vakatsiienko

Verified

This commit was signed with the committer’s verified signature.
dvakatsiienko Dima Vakatsiienko
… required by jest

Verified

This commit was signed with the committer’s verified signature.
dvakatsiienko Dima Vakatsiienko

Verified

This commit was signed with the committer’s verified signature.
dvakatsiienko Dima Vakatsiienko
@dvakatsiienko dvakatsiienko force-pushed the update-with-redux-example branch from 36589e1 to f0b6bfe Compare June 15, 2023 15:28

Verified

This commit was signed with the committer’s verified signature.
dvakatsiienko Dima Vakatsiienko
@dvakatsiienko dvakatsiienko force-pushed the update-with-redux-example branch from 329759a to 267ed21 Compare June 15, 2023 15:46
@dvakatsiienko
Copy link
Contributor Author

@leerob I've removed tests and sass, see if all good now.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
Copy link
Member

@balazsorban44 balazsorban44 left a comment

Choose a reason for hiding this comment

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

Let's drop src as well, to match other examples.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
Copy link
Member

@balazsorban44 balazsorban44 left a comment

Choose a reason for hiding this comment

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

Let's not add barrel files (index.ts files that are just re-exports), feels like adding complexity instead of reducing it.

balazsorban44 and others added 3 commits June 15, 2023 18:29

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.

Verified

This commit was signed with the committer’s verified signature.
dvakatsiienko Dima Vakatsiienko
@dvakatsiienko dvakatsiienko force-pushed the update-with-redux-example branch from 68a80b6 to 53f5d6e Compare June 15, 2023 21:14

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
@dvakatsiienko
Copy link
Contributor Author

@balazsorban44 removed src folder, and barrel files.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
@ijjk
Copy link
Member

ijjk commented Jun 16, 2023

Allow CI Workflow Run

  • approve CI run for commit: eec7825

Note: this should only be enabled once the PR is ready to go and can only be enabled by a maintainer

Sorry, something went wrong.

@ijjk
Copy link
Member

ijjk commented Jun 16, 2023

Allow CI Workflow Run

  • approve CI run for commit: eec7825

Note: this should only be enabled once the PR is ready to go and can only be enabled by a maintainer

Sorry, something went wrong.

@kodiakhq kodiakhq bot merged commit 04cd1fd into vercel:canary Jun 16, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 16, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
examples Issue was opened via the examples template. locked
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants