1. Created workflow file: .github/workflows/deploy-demo.yml:1#9
Conversation
- Builds the library first
- Builds the demo with the library source bundled in
- Deploys to GitHub Pages
- Triggers on pushes to master branch and can be manually triggered
2. Added build script: package.json:25
- Added build:demo script for building the demo locally
Reviewer's GuideThis PR introduces a new GitHub Actions workflow to build and deploy the demo to GitHub Pages, updates existing CI workflows to use Node 22 and enhanced linting/auditing steps, and adds local demo build support along with dependency and ESLint configuration updates. Class diagram for updated package.json scriptsclassDiagram
class PackageJson {
scripts: clean
scripts: build
scripts: build:demo
scripts: typecheck
scripts: format
scripts: lint
scripts: test
scripts: test:watch
scripts: test:coverage
scripts: test:ci
scripts: prepare
}
class BuildDemoScript {
command: "vite build --config demo/vite.config.ts"
}
PackageJson "1" -- "1" BuildDemoScript: contains
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Summary of ChangesHello @hbmartin, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the project's development and deployment infrastructure. It introduces an automated continuous deployment pipeline for the demo application to GitHub Pages, simplifies the local demo build process, and improves code quality enforcement through updated ESLint configurations and new plugins. These changes collectively aim to create a more efficient and robust development workflow. Highlights
Ignored Files
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
WalkthroughIntroduces a new GitHub Pages deployment workflow, updates Node.js versions in CI (release and tests) to 22.x, restructures the tests workflow, ignores root-level dist in .gitignore, enhances ESLint configuration with promise and jest plugins, and updates package.json scripts, repository URL, and devDependencies. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Dev as Push/Dispatch
participant GA as GitHub Actions
participant Build as Job: build
participant Deploy as Job: deploy
participant Pages as GitHub Pages
Dev->>GA: push to master / workflow_dispatch
GA->>Build: start (ubuntu-latest)
Build->>Build: checkout repo
Build->>Build: setup-node (Node 20, yarn cache)
Build->>Build: yarn install --frozen-lockfile
Build->>Build: yarn build (library)
Build->>Build: yarn vite build (demo) with base path
Build->>GA: upload artifact (demo dist)
GA->>Deploy: start after build
Deploy->>Pages: actions/deploy-pages@v4
Pages-->>Deploy: page_url output
note right of Deploy: Exposes page URL via outputs
sequenceDiagram
autonumber
participant Dev as Push/PR
participant GA as GitHub Actions
participant Test as Job: tests
Dev->>GA: trigger tests workflow
GA->>Test: start (ubuntu-latest)
Test->>Test: setup-node@v4 (Node 22.x, yarn cache)
Test->>Test: yarn --frozen-lockfile
Test->>Test: yarn audit
Test->>Test: prettier --check src/**/*.{ts,tsx,js,jsx,json,css,md}
Test->>Test: yarn test
Test->>Test: yarn lint
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
PR Compliance Guide 🔍Below is a summary of compliance checks for this PR:
Compliance status legend🟢 - Fully Compliant🟡 - Partial Compliant 🔴 - Not Compliant ⚪ - Requires Further Human Verification 🏷️ - Compliance label |
|||||||||||||||||||
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
There was a problem hiding this comment.
Hey there - I've reviewed your changes - here's some feedback:
- Use the new build:demo npm script in deploy-demo.yml instead of repeating the vite build command to avoid duplication.
- Standardize the Node.js version across workflows (tests, release, deploy-demo) to prevent inconsistencies; e.g., use 22.x everywhere.
- You added the 'publint' dependency but didn't configure it in eslint.config.ts—either integrate it into the ESLint plugins or remove the unused dependency.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Use the new build:demo npm script in deploy-demo.yml instead of repeating the vite build command to avoid duplication.
- Standardize the Node.js version across workflows (tests, release, deploy-demo) to prevent inconsistencies; e.g., use 22.x everywhere.
- You added the 'publint' dependency but didn't configure it in eslint.config.ts—either integrate it into the ESLint plugins or remove the unused dependency.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
Looks like there are a few issues preventing this PR from being merged!
If you'd like me to help, just leave a comment, like Feel free to include any additional details that might help me get this PR into a better state. You can manage your notification settings |
PR Code Suggestions ✨Explore these optional code suggestions:
|
||||||||||||||
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed everything up to 1a5886e in 1 minute and 41 seconds. Click for details.
- Reviewed
185lines of code in5files - Skipped
2files when reviewing. - Skipped posting
7draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. .github/workflows/deploy-demo.yml:28
- Draft comment:
Node.js version '20' here differs from '22.x' used in other workflows. Consider aligning them for consistency. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 20% vs. threshold = 50% 1. This is a new file being added, so any changes would be relevant. 2. The comment claims there's inconsistency with other workflows, but I can't verify this claim without seeing the other files. 3. Node.js 20 is a valid LTS version and perfectly reasonable choice. 4. Version consistency across workflows is nice-to-have but not critical - different workflows might have different requirements. I might be too quick to dismiss version consistency - it could lead to subtle issues if different parts of the codebase are tested/built with different Node versions. While consistency is good, without seeing the other workflows, we can't verify if this is actually an issue or if Node 20 was chosen deliberately for this specific workflow. Delete this comment because we lack the context to verify if there's actually an inconsistency, and Node.js 20 is a valid choice on its own.
2. .github/workflows/deploy-demo.yml:37
- Draft comment:
Consider using the 'build:demo' script from package.json instead of directly invoking Vite, unless the added '--base' flag is intentional. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 20% vs. threshold = 50% 1. The --base flag is crucial for GitHub Pages deployment to work correctly. 2. We don't have visibility into package.json to know if build:demo exists or what it does. 3. Even if build:demo exists, it might not include the necessary --base flag. 4. The current command is explicit and clear about what it's doing. 5. This seems like a preference suggestion rather than a necessary change. Without seeing package.json, we can't be certain whether build:demo would be better or worse. The current command might be duplicating existing configuration. The explicit command with --base flag shows clear intention for GitHub Pages deployment, and we can't assume a package.json script would handle this correctly. Delete the comment as it's speculative without package.json context and the current command is clearly intentional for GitHub Pages deployment.
3. eslint.config.ts:6
- Draft comment:
Ensure that the rules from eslint-plugin-promise integrate well with your existing async and code-quality rules. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
4. eslint.config.ts:15
- Draft comment:
Verify that the newly added jest plugin configuration does not conflict with your test file linting rules. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
5. package.json:25
- Draft comment:
Review the 'build:demo' script for consistency with the workflow command, as the workflow uses an extra '--base' flag. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50% 1. The comment references a workflow that isn't shown in the diff or context. 2. Without seeing the workflow file, we can't verify if this inconsistency actually exists. 3. The '--base' flag in Vite is for setting the base public path, which is a deployment concern. 4. Making assumptions about workflow requirements without evidence violates our review rules. I might be missing some important CI/CD context that could make this flag necessary. The workflow file might exist elsewhere. Even if there is a workflow file somewhere, we're instructed to ignore cross-file issues and only keep comments with strong evidence in the current diff. Delete this comment as it makes assumptions about workflow requirements without clear evidence in the current diff.
6. package.json:40
- Draft comment:
Repository URL now correctly uses 'https'. Ensure this update is reflected in all related documentation. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%The comment is asking the PR author to ensure that documentation is updated, which is against the rules. It doesn't provide a specific code suggestion or point out a potential issue with the code itself.
7. package.json:87
- Draft comment:
ESLint dependency updated to ^9.37.0. Confirm that your configuration remains compatible with this newer version. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%The comment is about a dependency update, specifically ESLint. It asks the author to confirm compatibility with the new version. According to the rules, comments on dependency changes or asking for confirmation are not allowed. Therefore, this comment should be removed.
Workflow ID: wflow_iLn8l4ujupLfD40u
You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.
There was a problem hiding this comment.
Code Review
This pull request introduces a GitHub Actions workflow for deploying the demo to GitHub Pages, updates Node.js versions in other workflows, and enhances the ESLint configuration by adding eslint-plugin-promise and eslint-plugin-jest. The changes are generally good, but I've provided a couple of suggestions in eslint.config.ts to improve code style and consistency by sorting imports and plugin definitions, adhering to the project's own linting rules.
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
.github/workflows/deploy-demo.yml (1)
37-38: Consider using the build:demo script for consistency.While the inline command works, using the
build:demoscript from package.json would provide better consistency and make the workflow easier to maintain.Apply this diff:
- name: Build demo - run: yarn vite build --config demo/vite.config.ts --base=/react-mentions-ts/ + run: yarn build:demo --base=/react-mentions-ts/Note: Verify that the vite CLI accepts the
--baseflag when run through the package.json script.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (6)
.github/workflows/deploy-demo.yml(1 hunks).github/workflows/release.yml(1 hunks).github/workflows/tests.yml(1 hunks).gitignore(1 hunks)eslint.config.ts(3 hunks)package.json(4 hunks)
🧰 Additional context used
🪛 GitHub Actions: CI
eslint.config.ts
[error] 1-1: ESLint Resolve error: typescript with invalid interface loaded as resolver (import/namespace).
package.json
[warning] 1-1: ESLint was unable to complete due to the above errors; there may be additional warnings in the full log.
🪛 GitHub Check: build
eslint.config.ts
[failure] 15-15:
eslint-plugin-jest import should occur before import of eslint-plugin-prettier/recommended
[failure] 44-44:
Unsafe member access .configs on an error typed value
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Sourcery review
🔇 Additional comments (11)
.gitignore (1)
12-13: LGTM!The additions are appropriate:
/.ideacorrectly ignores IDE-specific filesdistat root level complements the existing/demo/distignore and aligns with the new build outputseslint.config.ts (1)
44-52: Verify compatibility of new ESLint plugins.The added plugins (eslint-plugin-promise@7.2.1 and eslint-plugin-jest@29.0.1) both support flat config as confirmed by the learnings. However, ensure they're compatible with ESLint ^9.37.0 and the other plugins in your configuration.
Based on learnings: Both plugins support flat/recommended configs and are at their latest versions. However, the pipeline failure suggests there may be a compatibility issue with the import resolver.
package.json (3)
25-25: LGTM! Build script aligns with deployment workflow.The
build:demoscript is correctly referenced by the deploy-demo workflow (line 38 of deploy-demo.yml uses the inline command, but this script provides a convenient shorthand for local development).
40-40: Good catch fixing the malformed repository URL.The previous
git+https@...syntax was invalid. The correctedgit+https://...format is proper.
87-87: Dependency additions align with ESLint configuration changes.The new dependencies are appropriate:
eslint-plugin-jest@29.0.1andeslint-plugin-promise@7.2.1match the imports in eslint.config.tspublint@0.3.14is a useful tool for validating package exportsBased on learnings: Both ESLint plugins are at their latest stable versions and support the flat config format being used.
Also applies to: 91-91, 94-94, 106-106
.github/workflows/tests.yml (2)
8-13: Excellent workflow improvements.The new setup is clearer and adds valuable checks:
- Node.js 22.x aligns with the release workflow
- Yarn caching improves CI performance
- Explicit cache-dependency-path ensures cache invalidation on lockfile changes
14-18: Good additions: audit and prettier checks.The workflow now includes:
yarn auditfor security vulnerability scanning- Prettier format checking before tests
- Explicit test and lint steps
These additions improve code quality gates and security posture.
.github/workflows/deploy-demo.yml (3)
9-16: LGTM! Appropriate permissions and concurrency configuration.The workflow correctly:
- Grants minimal required permissions (contents: read, pages: write, id-token: write)
- Uses a concurrency group to prevent simultaneous deployments
- Disables cancel-in-progress to avoid partial deployments
19-46: Workflow structure is sound.The build job correctly:
- Checks out code and sets up dependencies
- Builds the library before the demo (correct dependency order)
- Configures GitHub Pages and uploads the artifact from the correct path
48-57: Deployment job is correctly configured.The deploy job:
- Properly depends on the build job
- Uses the standard actions/deploy-pages action
- Exposes the deployment URL via outputs
.github/workflows/release.yml (1)
22-22: Standardize Node.js version across workflows.The inconsistency is confirmed:
tests.ymlandrelease.ymlboth specify Node.js 22.x, whiledeploy-demo.ymluses Node.js 20. Verify whether the deploy-demo workflow requires Node.js 20 for specific reasons; if not, upgrade it to 22.x for consistency across all workflows.
Fixes #ABC
What did you change (functionally and technically)?
Checklist (remove this list before you submit the PR)
Additional review hints (remove this list before you submit the PR)
Summary by Sourcery
Configure CI workflows to use Node.js 22 with caching, extend linting and auditing steps, and add a dedicated workflow for building and deploying the demo to GitHub Pages along with a new local build script.
New Features:
Enhancements:
CI:
Summary by CodeRabbit
New Features
Tests
Chores