Skip to content

fix(types): should accept Child and Properties at the same time#24

Open
JounQin wants to merge 1 commit into
mainfrom
fix/types
Open

fix(types): should accept Child and Properties at the same time#24
JounQin wants to merge 1 commit into
mainfrom
fix/types

Conversation

@JounQin
Copy link
Copy Markdown
Member

@JounQin JounQin commented Mar 19, 2026

Initial checklist

  • I read the support docs
  • I read the contributing guide
  • I agree to follow the code of conduct
  • I searched issues and discussions and couldn’t find anything or linked relevant results below
  • I made sure the docs are up to date
  • I included tests (or that’s not needed)

Description of changes

The second parameter accepts Child or Properties at the same time actually, and when using node.attributes via remark-directive, there is no need to add || {} at

https://github.com/remarkjs/remark-directive/blob/23b8f416da165b6ddaa4c5e7e82addaf6dcb96a9/readme.md?plain=1#L161

Copilot AI review requested due to automatic review settings March 19, 2026 08:35
@github-actions github-actions Bot added 👋 phase/new Post is being triaged automatically 🤞 phase/open Post is being triaged manually and removed 👋 phase/new Post is being triaged automatically labels Mar 19, 2026
@JounQin JounQin requested review from remcohaszing and wooorm and removed request for Copilot March 19, 2026 08:36
@JounQin JounQin requested review from Copilot and removed request for wooorm March 19, 2026 14:39
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR adjusts the TypeScript surface of h/s so their second argument can be typed as a union of Child and Properties (matching actual runtime behavior), which helps consumers that pass values like node.attributes from remark-directive without extra casts/defaults.

Changes:

  • Update JSDoc overload in createH to accept Child | Properties as the second parameter.
  • Add a tsd type test that verifies a Child | Properties-typed value is accepted as the second argument to h.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
test-d/index.ts Adds a type-level regression test for passing a Child | Properties union as the second argument.
lib/create-h.js Updates the overload signature to reflect that the second parameter can be either Child or Properties.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

@wooorm
Copy link
Copy Markdown
Member

wooorm commented Jun 3, 2026

Yeah, but the next @overload below it supports it already? 🤔

   * @overload
   * @param {string} selector
   * @param {...Child} children
   * @returns {Element}

Are you sure your tests don’t pass, without the change?

Sorry for the wait!

@JounQin
Copy link
Copy Markdown
Member Author

JounQin commented Jun 3, 2026

@wooorm Yes, I'm sure, here the key point is Properties doesn't allow null | undefined, so null-able second parameter won't work which makes the workaround || {}.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🤞 phase/open Post is being triaged manually

Development

Successfully merging this pull request may close these issues.

4 participants