Drop support for node 18, move tests #3342
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 WalkthroughWalkthroughConsolidates test workspaces under Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
shepherd.js/test/cypress/integration/test.acceptance.cy.js (1)
1-10: Add Cypress globals declaration to test file.The test file uses Cypress globals (
cy,describe,beforeEach) that ESLint reports as undefined. The project's ESLint configuration lacks an override for cypress test files, so add the in-file directive to declare them.Fix
+/* eslint-env cypress, mocha */ import setupTour from '../utils/setup-tour'; import { assert } from 'chai';shepherd.js/test/cypress/integration/modal.cy.js (1)
1-10: Fix ESLintno-undeffor Cypress globals after test move.Line 10 now triggers
cy is not definedin ESLint; ensure Cypress globals apply to this new path or declare them in-file to keep CI green.🛠️ Minimal file-level fix
+/* global cy */ import setupTour from '../utils/setup-tour'; import { expect } from 'chai';
🧹 Nitpick comments (2)
shepherd.js/test/unit/components/shepherd-modal.spec.js (1)
535-544: Inconsistent mocking API in skipped tests.These skipped tests use
jest.spyOn(lines 535-540, 566-571), but the file importsvifrom Vitest (line 1). If these tests are re-enabled, they will fail becausejestis not defined.🔧 Suggested fix when re-enabling tests
- hideStub = jest + hideStub = vi .spyOn(modalComponent, 'hide') .mockImplementation(() => {}); - showStub = jest + showStub = vi .spyOn(modalComponent, 'show') .mockImplementation(() => {});shepherd.js/package.json (1)
49-50: Minor redundancy:test:allandtest:ciare identical.Both scripts execute the same commands. Consider removing one to reduce confusion, or document their intended semantic difference if one exists.
♻️ Suggested cleanup
"test": "vitest", - "test:all": "pnpm test:unit:ci && pnpm test:cy:ci", "test:ci": "pnpm test:unit:ci && pnpm test:cy:ci",
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
shepherd.js/test/unit/components/shepherd-modal.spec.js (1)
535-544: Incomplete Jest-to-Vitest migration:jest.spyOnshould bevi.spyOn.These skipped tests still use
jest.spyOn, but Jest is not imported—only Vitest is. If un-skipped, they will fail withReferenceError: jest is not defined. Importvifrom Vitest and replacejest.spyOnwithvi.spyOn.🔧 Proposed fix
Update the import on line 1:
-import { describe, expect, it } from 'vitest'; +import { describe, expect, it, vi } from 'vitest';Then replace
jest.spyOncalls:- hideStub = jest - .spyOn(modalComponent, 'hide') - .mockImplementation(() => {}); - showStub = jest - .spyOn(modalComponent, 'show') - .mockImplementation(() => {}); + hideStub = vi + .spyOn(modalComponent, 'hide') + .mockImplementation(() => {}); + showStub = vi + .spyOn(modalComponent, 'show') + .mockImplementation(() => {});
🤖 Fix all issues with AI agents
In `@shepherd.js/test/cypress/dummy/js/prism.js`:
- Around line 3-10: This vendored PrismJS bundle (contains top-level symbols
_self and Prism) should be excluded from linting: either add this file to the
project's lint ignore patterns (eg. update eslint/biome config ignorePatterns to
include shepherd.js/test/cypress/dummy/js/prism.js) or mark the file as
generated/vendor by adding the appropriate project convention (or move it into a
lint-ignored assets dir); do not change the bundle content—just update lint
configuration or relocate the file so tools skip linting it.
- Around line 1-2: Add the PrismJS MIT license header to the top of the vendored
prism.js file: insert a standard MIT attribution block mentioning PrismJS
(PrismJS 1.15.0), the original author (Lea Verou) and the MIT License
text/copyright notice before the existing comment that references the download
URL so the file clearly includes the required license header.
In `@shepherd.js/test/cypress/integration/css.cy.js`:
- Around line 8-27: Add Cypress globals to ESLint for this test file by enabling
the cypress environment: either add a top-of-file ESLint directive (e.g., a
single-line "eslint-env cypress" comment) to the file containing describe/it/cy
usages, or update your ESLint config overrides to include the test directory (so
calls to describe, it, cy, and cy.window are recognized); target the test file
that defines "describe('CSS Import Behavior', ...)" and the "it" blocks to avoid
undefined-variable lint errors.
🧹 Nitpick comments (2)
shepherd.js/test/unit/components/shepherd-modal.spec.js (1)
581-587: Component mounted outside test scope creates shared mutable state.
modalComponentis instantiated at describe-block level, making it shared across theshowandhidetests. This couples the tests (hide depends on show running first) and risks pollution if more tests are added. Consider moving the mount into abeforeEachor into eachitblock for isolation.♻️ Suggested refactor
describe('show/hide', function () { - const modalComponent = mount(ShepherdModal, { - target: document.body, - props: { - classPrefix - } - }); + let modalComponent; + + beforeEach(() => { + modalComponent = mount(ShepherdModal, { + target: document.body, + props: { + classPrefix + } + }); + }); + + afterEach(() => { + unmount(modalComponent); + }); it('show adds classes', async () => { await modalComponent.show(); expect(modalComponent.getElement()).toHaveClass( 'shepherd-modal-is-visible' ); }); it('hide removes classes', async () => { + await modalComponent.show(); // ensure visible first await modalComponent.hide(); expect(modalComponent.getElement()).not.toHaveClass( 'shepherd-modal-is-visible' ); - - unmount(modalComponent); }); });shepherd.js/test/cypress/integration/modal.cy.js (1)
72-82: Avoidasync/awaitwith Cypress command chains.Cypress commands are queued and not native Promises. Using
awaitbreaks the command chain and can cause ordering surprises wheretour.hide()executes before the preceding Cypress command completes. Use.then()and closures for proper sequencing instead.Suggested rewrite
- it('removes shepherd-modal-is-visible class from the overlay', async () => { + it('removes shepherd-modal-is-visible class from the overlay', () => { tour.start(); - await cy - .get('.shepherd-modal-overlay-container') - .should('have.class', 'shepherd-modal-is-visible'); + cy.get('.shepherd-modal-overlay-container') + .should('have.class', 'shepherd-modal-is-visible') + .then(() => { + tour.hide(); + }); - tour.hide(); cy.get('.shepherd-modal-overlay-container').should( 'not.have.class', 'shepherd-modal-is-visible' ); });
| /* PrismJS 1.15.0 | ||
| https://prismjs.com/download.html#themes=prism&languages=markup+clike+javascript&plugins=unescaped-markup+normalize-whitespace */ |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Explore repository root for license/attribution files
find . -maxdepth 2 -type f \( -name "LICENSE*" -o -name "NOTICE*" -o -name "COPYING*" -o -name "AUTHORS*" -o -name "CREDITS*" \) 2>/dev/null | head -20Repository: shipshapecode/shepherd
Length of output: 78
🏁 Script executed:
# Check if PrismJS is mentioned in any existing license/attribution files
rg -i "prism" --type txt --type md . 2>/dev/null | head -30Repository: shipshapecode/shepherd
Length of output: 48
🏁 Script executed:
# Verify the file exists and check its actual content (first 10 lines)
cat -n shepherd.js/test/cypress/dummy/js/prism.js 2>/dev/null | head -15Repository: shipshapecode/shepherd
Length of output: 698
🏁 Script executed:
# Check for any lint ignore configurations that might affect this file
find . -maxdepth 3 -type f \( -name ".eslintignore" -o -name ".eslintrc*" -o -name "eslint.config.*" \) 2>/dev/null | xargs grep -l "prism\|cypress/dummy" 2>/dev/nullRepository: shipshapecode/shepherd
Length of output: 81
🌐 Web query:
PrismJS 1.15.0 license requirements MIT copyright attribution
💡 Result:
PrismJS v1.15.0 is released under the MIT License. [1][2]
What you must do (MIT attribution requirements)
When you redistribute PrismJS (source or bundled/minified “substantial portions”), you must include both:
- the copyright notice, and
- the MIT permission + warranty disclaimer text
in your distribution (commonly by shipping aLICENSEfile and/or retaining the header in distributed files). [3]
Copyright / attribution text to preserve
Prism’s license notice begins with:
MIT LICENSE Copyright (c) 2012 Lea Verou[3]
Sources:
[1] https://www.npmjs.com/package/prismjs/v/1.15.0
[2] https://cdnjs.com/libraries/prism/1.15.0
[3] https://cdn.jsdelivr.net/gh/PrismJS/prism%401.30.0/LICENSE
🏁 Script executed:
# Check LICENSE.md for PrismJS attribution
cat LICENSE.mdRepository: shipshapecode/shepherd
Length of output: 36983
🏁 Script executed:
# Check .eslintignore to see if prism.js is excluded
cat .eslintignoreRepository: shipshapecode/shepherd
Length of output: 163
Add PrismJS MIT license header to the vendored bundle.
This file vendors PrismJS 1.15.0 (confirmed MIT Licensed), but the header only references the download URL without including the MIT copyright notice or license text. Although this is a test fixture in cypress/dummy/, adding the proper MIT attribution header (Copyright (c) 2012 Lea Verou ... MIT License) maintains compliance with PrismJS's license requirements and is a best practice even for test assets.
🤖 Prompt for AI Agents
In `@shepherd.js/test/cypress/dummy/js/prism.js` around lines 1 - 2, Add the
PrismJS MIT license header to the top of the vendored prism.js file: insert a
standard MIT attribution block mentioning PrismJS (PrismJS 1.15.0), the original
author (Lea Verou) and the MIT License text/copyright notice before the existing
comment that references the download URL so the file clearly includes the
required license header.
| var _self = | ||
| 'undefined' != typeof window | ||
| ? window | ||
| : 'undefined' != typeof WorkerGlobalScope && | ||
| self instanceof WorkerGlobalScope | ||
| ? self | ||
| : {}, | ||
| Prism = (function () { |
There was a problem hiding this comment.
Exclude this vendored bundle from linting to avoid CI failures.
Static analysis already flags multiple ESLint/Biome errors (no-undef, no-redeclare, no-prototype-builtins, etc.) that are expected for a raw PrismJS bundle. Mark this file as vendor/generated and exclude it from linting (or move it to a lint-ignored asset path) to prevent build breaks.
🧰 Tools
🪛 ESLint
[error] 7-7: 'WorkerGlobalScope' is not defined.
(no-undef)
🤖 Prompt for AI Agents
In `@shepherd.js/test/cypress/dummy/js/prism.js` around lines 3 - 10, This
vendored PrismJS bundle (contains top-level symbols _self and Prism) should be
excluded from linting: either add this file to the project's lint ignore
patterns (eg. update eslint/biome config ignorePatterns to include
shepherd.js/test/cypress/dummy/js/prism.js) or mark the file as generated/vendor
by adding the appropriate project convention (or move it into a lint-ignored
assets dir); do not change the bundle content—just update lint configuration or
relocate the file so tools skip linting it.
| describe('CSS Import Behavior', () => { | ||
| it('includes project styles when explicitly imported', () => { | ||
| cy.visit('/examples/css/with-css-import'); | ||
| cy.visit('/test/cypress/examples/css/with-css-import'); | ||
| cy.window().should('have.property', 'Shepherd'); | ||
| cy.get('[data-test-id="fake-modal-overlay"]').should('have.css', 'pointer-events', 'none'); | ||
| cy.get('[data-test-id="fake-modal-overlay"]').should( | ||
| 'have.css', | ||
| 'pointer-events', | ||
| 'none' | ||
| ); | ||
| }); | ||
|
|
||
| it('DOES NOT include project styles without explicit import', () => { | ||
| cy.visit('/examples/css/no-css-import'); | ||
| cy.visit('/test/cypress/examples/css/no-css-import'); | ||
| cy.window().should('have.property', 'Shepherd'); | ||
| cy.get('[data-test-id="fake-modal-overlay"]').should('not.have.css', 'pointer-events', 'none'); | ||
| cy.get('[data-test-id="fake-modal-overlay"]').should( | ||
| 'not.have.css', | ||
| 'pointer-events', | ||
| 'none' | ||
| ); | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, let's find ESLint config files and check for Cypress settings
find . -maxdepth 3 -type f \( -name 'eslint.config.*' -o -name '.eslintrc*' -o -name 'package.json' \) | head -20Repository: shipshapecode/shepherd
Length of output: 227
🏁 Script executed:
#!/bin/bash
# Check if the test file exists
find . -path "*test/cypress/integration/css.cy.js" -o -path "*test/cypress/*/css.cy.js" 2>/dev/nullRepository: shipshapecode/shepherd
Length of output: 114
🏁 Script executed:
#!/bin/bash
# Search for Cypress-related ESLint configuration patterns
rg -i "cypress|globals.*cy|env.*cypress" --type json --type js -n 2>/dev/null | head -40Repository: shipshapecode/shepherd
Length of output: 2728
🏁 Script executed:
#!/bin/bash
cat -n ./.eslintrc.jsRepository: shipshapecode/shepherd
Length of output: 1648
🏁 Script executed:
#!/bin/bash
cat -n ./shepherd.js/.eslintrc.cjsRepository: shipshapecode/shepherd
Length of output: 1809
🏁 Script executed:
#!/bin/bash
# Check if any .cy.js files already have the eslint-env cypress fix
rg -l "eslint-env cypress" shepherd.js/test/cypress/Repository: shipshapecode/shepherd
Length of output: 48
🏁 Script executed:
#!/bin/bash
# Show the first few lines of other .cy.js files to see if they have the fix
head -5 shepherd.js/test/cypress/integration/test.acceptance.cy.js
head -5 shepherd.js/test/cypress/integration/a11y.cy.js
head -5 shepherd.js/test/cypress/integration/modal.cy.jsRepository: shipshapecode/shepherd
Length of output: 375
Configure Cypress globals in ESLint for the test directory.
ESLint does not have Cypress environment settings for shepherd.js/test/cypress/ — it will flag cy, describe, it as undefined. Either add a local /* eslint-env cypress */ comment at the top of this file, or update the ESLint config override to cover the Cypress test directory.
Example local fix (quickest approach)
+/* eslint-env cypress */
/*
* The `.shepherd-modal-overlay-container` class is chosen for inspection
* because the Svelte styles for the modal are added to the import graph as🧰 Tools
🪛 ESLint
[error] 8-8: 'describe' is not defined.
(no-undef)
[error] 9-9: 'it' is not defined.
(no-undef)
[error] 10-10: 'cy' is not defined.
(no-undef)
[error] 11-11: 'cy' is not defined.
(no-undef)
[error] 12-12: 'cy' is not defined.
(no-undef)
[error] 19-19: 'it' is not defined.
(no-undef)
[error] 20-20: 'cy' is not defined.
(no-undef)
[error] 21-21: 'cy' is not defined.
(no-undef)
[error] 22-22: 'cy' is not defined.
(no-undef)
🤖 Prompt for AI Agents
In `@shepherd.js/test/cypress/integration/css.cy.js` around lines 8 - 27, Add
Cypress globals to ESLint for this test file by enabling the cypress
environment: either add a top-of-file ESLint directive (e.g., a single-line
"eslint-env cypress" comment) to the file containing describe/it/cy usages, or
update your ESLint config overrides to include the test directory (so calls to
describe, it, cy, and cy.window are recognized); target the test file that
defines "describe('CSS Import Behavior', ...)" and the "it" blocks to avoid
undefined-variable lint errors.
Summary by CodeRabbit
Chores
Tests
Chores
New Features
✏️ Tip: You can customize this high-level summary in your review settings.