Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[react-dom] Update types for latest state of Float #66896

Merged
merged 5 commits into from Oct 4, 2023

Conversation

@typescript-bot typescript-bot added this to Needs Author Action in New Pull Request Status Board Oct 2, 2023
@DangerBotOSS
Copy link

DangerBotOSS commented Oct 2, 2023

Inspecting the JavaScript source for this package found some properties that are not in the .d.ts files.
The check for missing properties isn't always right, so take this list as advice, not a requirement.

react-dom (unpkg)

was missing the following properties:

  1. createRoot
  2. hydrateRoot

Generated by 🚫 dangerJS against 20aff78

@eps1lon eps1lon marked this pull request as ready for review October 2, 2023 13:35
@typescript-bot
Copy link
Contributor

typescript-bot commented Oct 2, 2023

@eps1lon Thank you for submitting this PR!

This is a live comment which I will keep updated.

1 package in this PR

Code Reviews

Because this is a widely-used package, a DT maintainer will need to review it before it can be merged.

You can test the changes of this PR in the Playground.

Status

  • ✅ No merge conflicts
  • ✅ Continuous integration tests have passed
  • 🕐 Most recent commit is approved by a DT maintainer

Once every item on this list is checked, I'll ask you for permission to merge and publish the changes.


Diagnostic Information: What the bot saw about this PR
{
  "type": "info",
  "now": "-",
  "pr_number": 66896,
  "author": "eps1lon",
  "headCommitOid": "20aff78030b143e9bf74a3768b2e71044f1118ed",
  "mergeBaseOid": "98e68ce4add1e1fc4aad17402ae8eaab9ca15088",
  "lastPushDate": "2023-10-02T12:59:44.000Z",
  "lastActivityDate": "2023-10-04T18:42:34.000Z",
  "hasMergeConflict": false,
  "isFirstContribution": false,
  "tooManyFiles": false,
  "hugeChange": false,
  "popularityLevel": "Critical",
  "pkgInfo": [
    {
      "name": "react-dom",
      "kind": "edit",
      "files": [
        {
          "path": "types/react-dom/canary.d.ts",
          "kind": "definition"
        },
        {
          "path": "types/react-dom/test/canary-tests.tsx",
          "kind": "test"
        }
      ],
      "owners": [
        "MartynasZilinskas",
        "theruther4d",
        "Jessidhia",
        "eps1lon"
      ],
      "addedOwners": [],
      "deletedOwners": [],
      "popularityLevel": "Critical"
    }
  ],
  "reviews": [
    {
      "type": "approved",
      "reviewer": "gnoff",
      "date": "2023-10-04T18:42:34.000Z",
      "isMaintainer": false
    },
    {
      "type": "stale",
      "reviewer": "rbuckton",
      "date": "2023-10-03T20:02:55.000Z",
      "abbrOid": "fd29081"
    }
  ],
  "mainBotCommentID": 1743031004,
  "ciResult": "pass"
}

@typescript-bot typescript-bot added Critical package Author is Owner The author of this PR is a listed owner of the package. labels Oct 2, 2023
@typescript-bot typescript-bot moved this from Needs Author Action to Waiting for Code Reviews in New Pull Request Status Board Oct 2, 2023
@typescript-bot
Copy link
Contributor

🔔 @MartynasZilinskas @theruther4d @Jessidhia — please review this PR in the next few days. Be sure to explicitly select Approve or Request Changes in the GitHub UI so I know what's going on.

@typescript-bot typescript-bot moved this from Waiting for Code Reviews to Needs Maintainer Review in New Pull Request Status Board Oct 2, 2023
@typescript-bot typescript-bot moved this from Needs Maintainer Review to Waiting for Code Reviews in New Pull Request Status Board Oct 2, 2023
@typescript-bot typescript-bot moved this from Waiting for Code Reviews to Needs Maintainer Review in New Pull Request Status Board Oct 2, 2023
@typescript-bot typescript-bot added Maintainer Approved Self Merge This PR can now be self-merged by the PR author or an owner labels Oct 3, 2023
@typescript-bot typescript-bot moved this from Needs Maintainer Review to Waiting for Author to Merge in New Pull Request Status Board Oct 3, 2023
}
function preconnect(href: string, options?: PreconnectOptions): void;

type PreloadAs = "font" | "image" | "script" | "style";
Copy link
Contributor

Choose a reason for hiding this comment

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

Internally we just treat this as a string argument now however what probably make sense in TS is to enumerate all of the known valid as types for preloads

// from: https://developer.mozilla.org/en-US/docs/Web/HTML/Attributes/rel/preload#what_types_of_content_can_be_preloaded

audio
document
embed
fetch
object
script
style
track
worker
video

*/
as: PreloadModuleAs;
crossOrigin?: "anonymous" | "use-credentials" | "" | undefined;
integrity?: string | undefined;
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
integrity?: string | undefined;
integrity?: string | undefined;
nonce?: string | undefined;

*/
as?: PreinitModuleAs;
crossOrigin?: "anonymous" | "use-credentials" | "" | undefined;
integrity?: string | undefined;
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
integrity?: string | undefined;
integrity?: string | undefined;
nonce?: string | undefined;

@typescript-bot typescript-bot added Revision needed This PR needs code changes before it can be merged. and removed Self Merge This PR can now be self-merged by the PR author or an owner Maintainer Approved labels Oct 4, 2023
@typescript-bot typescript-bot moved this from Waiting for Author to Merge to Needs Author Action in New Pull Request Status Board Oct 4, 2023
@typescript-bot
Copy link
Contributor

@eps1lon One or more reviewers has requested changes. Please address their comments. I'll be back once they sign off or you've pushed new commits. Thank you!

---------

Co-authored-by: Josh Story <josh.c.story@gmail.com>
@typescript-bot typescript-bot removed the Revision needed This PR needs code changes before it can be merged. label Oct 4, 2023
@typescript-bot typescript-bot moved this from Needs Author Action to Waiting for Code Reviews in New Pull Request Status Board Oct 4, 2023
@typescript-bot typescript-bot added the Other Approved This PR was reviewed and signed-off by a community member. label Oct 4, 2023
@typescript-bot
Copy link
Contributor

@rbuckton Thank you for reviewing this PR! The author has pushed new commits since your last review. Could you take another look and submit a fresh review?

@typescript-bot typescript-bot moved this from Waiting for Code Reviews to Needs Maintainer Review in New Pull Request Status Board Oct 4, 2023
@eps1lon eps1lon merged commit 5f36f8f into DefinitelyTyped:master Oct 4, 2023
2 checks passed
@eps1lon eps1lon deleted the feat/react-dom/preload branch October 4, 2023 18:59
@typescript-bot typescript-bot removed this from Needs Maintainer Review in New Pull Request Status Board Oct 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Author is Owner The author of this PR is a listed owner of the package. Critical package Other Approved This PR was reviewed and signed-off by a community member.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants