Skip to content

Add failing tests for #82 - #95

Open
Wendy Fouchter (Wendystraite) wants to merge 1 commit into
saasquatch:mainfrom
Wendystraite:tests_issue_82
Open

Add failing tests for #82#95
Wendy Fouchter (Wendystraite) wants to merge 1 commit into
saasquatch:mainfrom
Wendystraite:tests_issue_82

Conversation

@Wendystraite

@Wendystraite Wendy Fouchter (Wendystraite) commented Oct 11, 2025

Copy link
Copy Markdown
Contributor

Description of the change

Add tests for issue #82.

These tests "passes" for now and can be merged but will not pass when fixed (using vitest's test.fails). Edit: removed the test.fails usage.

The tests shows that when when molecule is first implicit created then explicitely created in strict mode then it doesn't share the same instance in a complex scenario involving toggling on and off the explicit molecule.

The good news :

Type of change

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation or Development tools (readme, specs, tests, code formatting)

Copilot AI review requested due to automatic review settings October 11, 2025 17:48
@changeset-bot

changeset-bot Bot commented Oct 11, 2025

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: 904b12f

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull Request Overview

Adds a targeted test suite for Issue #82 to verify instance sharing between implicitly and explicitly created molecules, particularly under Strict Mode behavior changes.

  • Adds three tests exercising explicit-then-implicit and implicit-then-explicit ordering, including a real-component toggle scenario.
  • Introduces render from @testing-library/react to support component-level testing.
  • Uses test.fails to mark the known failing path in Strict Mode (implicit then explicit).

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Comment thread src/react/useMolecule.test.tsx Outdated
Comment thread src/react/useMolecule.test.tsx Outdated
Comment thread src/react/useMolecule.test.tsx Outdated
Comment thread src/react/useMolecule.test.tsx Outdated
Comment thread src/react/useMolecule.test.tsx Outdated
Comment thread src/react/useMolecule.test.tsx Outdated

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull Request Overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 3 comments.


Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Comment thread src/react/useMolecule.test.tsx Outdated
Comment thread src/react/useMolecule.test.tsx Outdated
Comment thread src/react/useMolecule.test.tsx

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull Request Overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 4 comments.

Comment thread src/react/useMolecule.test.tsx
Comment on lines +1209 to +1212
let initialValue: string;
let instancesAfterFirst: number;
let aValue: string;
let bValue: string;

Copilot AI Nov 18, 2025

Copy link

Choose a reason for hiding this comment

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

The variables initialValue, aValue, and bValue are typed as string, but textContent returns string | null. With TypeScript strict mode enabled, this could cause a type error. Consider either:

  1. Changing the type to string | null, or
  2. Using a non-null assertion operator (!) since in this test context the elements will always have text content, or
  3. Using a default value with the nullish coalescing operator (?? "")

Copilot uses AI. Check for mistakes.
Comment thread src/react/useMolecule.test.tsx
Comment thread src/react/useMolecule.test.tsx
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants