fix: not call destroy on destroyed step

[remiHau]

Currently on function _setupElements of step, the destroy function is called if this.el is undefined but the destroy method set this.el to null. So function destroy is called also on destroyed element which trigger unnecessary 'destroy' event.

Another possible change would be to change the condition in _setupElements to check if this.el is either undefined or null.

This is linked with reported issue #3443

Summary by CodeRabbit

• New Features

  • Added a new isNull type-guard utility for reliably detecting null values.

• Bug Fixes

  • Improved step element setup/teardown handling to better account for null vs undefined, and to avoid redundant teardown when a step has already been destroyed.

• Tests

  • Expanded unit coverage to confirm teardown isn’t triggered again on already-destroyed steps, even if destroy is later replaced.

[vercel]

Someone is attempting to deploy a commit to the shipshapecode Team on Vercel.

A member of the Team first needs to authorize it.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: b7dc4250-64b9-449b-bffe-5a1ecbf9ca5d

📥 Commits

Reviewing files that changed from the base of the PR and between 98217ff and c7a9cc6.

📒 Files selected for processing (3)

• shepherd.js/src/step.ts
• shepherd.js/src/utils/type-check.ts
• shepherd.js/test/unit/step.spec.js

🚧 Files skipped from review as they are similar to previous changes (1)

• shepherd.js/test/unit/step.spec.js

---

📝 Walkthrough

Walkthrough

A new isNull type-guard utility is added to the type-check module. In step.ts, the _setupElements() guard is strengthened to check both undefined and null before invoking destroy. A unit test is added confirming the guard correctly prevents redundant destroy calls on already-destroyed steps.

Changes

Null-checking type guard and step initialization

Layer / File(s)	Summary
isNull type guard and step initialization guard shepherd.js/src/utils/type-check.ts, shepherd.js/src/step.ts, shepherd.js/test/unit/step.spec.js	New isNull type-guard utility exports a strict null check; step.ts imports it and applies both isUndefined and isNull in the _setupElements() guard to prevent invoking destroy when this.el is either undefined or null; a new test constructs a destroyed step and confirms _setupElements() does not call the replaced destroy spy.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~4 minutes

Poem

A clever rabbit wrote a guard so fine,
To catch the null and undefined design.
When el is neither here nor there,
destroy sleeps soundly, handled with care.
🐇✨ The step now awakens, twice as aware!

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'fix: not call destroy on destroyed step' directly and specifically addresses the main bug fix in this PR, which is preventing the destroy method from being called on already destroyed steps.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

ESLint install failed: private package registry requires authentication. Disable ESLint in CodeRabbit settings or use public packages.

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@shepherd.js/src/step.ts`:
- Line 450: The teardown at line 450 assigns `undefined` to this.el, but the
getElement() method's contract and type signature indicate it returns null for
destroyed steps. Revert line 450 to assign null instead of undefined to maintain
the documented contract. Then update the guard logic in _setupElements() to
check if this.el is either null or undefined (treating both as "not mounted") to
handle any edge cases where undefined might be present from other code paths.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 82196bde-61fb-41d7-8d5f-b3b9cfd8b837

📥 Commits

Reviewing files that changed from the base of the PR and between e256dcb and 98217ff.

📒 Files selected for processing (2)

• shepherd.js/src/step.ts
• shepherd.js/test/unit/step.spec.js