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

[relay-runtime] Move PreloadableConcreteRequest from react-relay to relay-runtime #67321

Conversation

tobias-tengler
Copy link
Contributor

@tobias-tengler tobias-tengler commented Nov 5, 2023

I'm working on adding @preloadable support to the OSS version of Relay: facebook/relay#4515
The generated $parameters.ts file needs to import the PreloadableConcreteRequest type. Currently this type is defined in @types/react-relay for seemingly no reason. The other request types are defined in @types/relay-runtime.
This PR moves PreloadableConcreteRequest to @types/relay-runtime so the relay-compiler doesn't have to suddenly start importing types from react-relay.

Please fill in this template.

Select one of these and delete the others:

If changing an existing definition:

@typescript-bot typescript-bot added this to Needs Author Action in New Pull Request Status Board Nov 5, 2023
@tobias-tengler tobias-tengler marked this pull request as ready for review November 5, 2023 19:52
@typescript-bot
Copy link
Contributor

typescript-bot commented Nov 5, 2023

@tobias-tengler Thank you for submitting this PR!

This is a live comment which I will keep updated.

2 packages in this PR

Code Reviews

Because this PR edits multiple packages, it can be merged once it's reviewed by a DT maintainer.

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

Status

  • ❌ No merge conflicts
  • ✅ Continuous integration tests have passed
  • 🕐 A DT maintainer needs to approve changes which affect more than one package

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

Inactive

This PR has been inactive for 31 days — it is considered abandoned, and therefore closed!


Diagnostic Information: What the bot saw about this PR
{
  "type": "info",
  "now": "-",
  "pr_number": 67321,
  "author": "tobias-tengler",
  "headCommitOid": "2be50889e44faa2dbcf7547fab562442711b6cc5",
  "mergeBaseOid": "39544fd1c88147d33653d7d414c4e4b746a8a955",
  "lastPushDate": "2023-11-05T19:46:46.000Z",
  "lastActivityDate": "2023-11-17T01:10:05.000Z",
  "hasMergeConflict": true,
  "isFirstContribution": false,
  "tooManyFiles": false,
  "hugeChange": false,
  "popularityLevel": "Popular",
  "pkgInfo": [
    {
      "name": "react-relay",
      "kind": "edit",
      "files": [
        {
          "path": "types/react-relay/hooks.d.ts",
          "kind": "definition"
        },
        {
          "path": "types/react-relay/relay-hooks/EntryPointTypes.d.ts",
          "kind": "definition"
        },
        {
          "path": "types/react-relay/relay-hooks/loadQuery.d.ts",
          "kind": "definition"
        },
        {
          "path": "types/react-relay/relay-hooks/useQueryLoader.d.ts",
          "kind": "definition"
        },
        {
          "path": "types/react-relay/v7/lib/relay-experimental/EntryPointTypes.d.ts",
          "kind": "definition"
        },
        {
          "path": "types/react-relay/v7/lib/relay-experimental/loadQuery.d.ts",
          "kind": "definition"
        },
        {
          "path": "types/react-relay/v7/lib/relay-experimental/useQueryLoader.d.ts",
          "kind": "definition"
        }
      ],
      "owners": [
        "alloy",
        "maraisr",
        "edvinerikson",
        "levibuzolic"
      ],
      "addedOwners": [],
      "deletedOwners": [],
      "popularityLevel": "Popular"
    },
    {
      "name": "relay-runtime",
      "kind": "edit",
      "files": [
        {
          "path": "types/relay-runtime/index.d.ts",
          "kind": "definition"
        },
        {
          "path": "types/relay-runtime/lib/util/RelayConcreteNode.d.ts",
          "kind": "definition"
        },
        {
          "path": "types/relay-runtime/relay-runtime-tests.tsx",
          "kind": "test"
        }
      ],
      "owners": [
        "alloy",
        "maraisr",
        "morrys",
        "levibuzolic"
      ],
      "addedOwners": [],
      "deletedOwners": [],
      "popularityLevel": "Popular"
    }
  ],
  "reviews": [
    {
      "type": "stale",
      "reviewer": "maraisr",
      "date": "2023-11-06T08:22:05.000Z",
      "abbrOid": "28d0f0c"
    }
  ],
  "mainBotCommentID": 1793829726,
  "ciResult": "pass"
}

@typescript-bot typescript-bot added Popular package This PR affects a popular package (as counted by NPM download counts). Edits multiple packages labels Nov 5, 2023
@typescript-bot typescript-bot moved this from Needs Author Action to Waiting for Code Reviews in New Pull Request Status Board Nov 5, 2023
@typescript-bot
Copy link
Contributor

🔔 @alloy @maraisr @edvinerikson @levibuzolic @morrys — 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
Copy link
Contributor

typescript-bot commented Nov 5, 2023

@tobias-tengler Thank you for submitting this PR!

This is a live comment which I will keep updated.

2 packages in this PR

Code Reviews

Because this PR edits multiple packages, it can be merged once it's reviewed by a DT maintainer.

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

Status

  • ❌ No merge conflicts
  • ✅ Continuous integration tests have passed
  • 🕐 A DT maintainer needs to approve changes which affect more than one package

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

Inactive

This PR has been inactive for 31 days — it is considered abandoned, and therefore closed!


Diagnostic Information: What the bot saw about this PR
{
  "type": "info",
  "now": "-",
  "pr_number": 67321,
  "author": "tobias-tengler",
  "headCommitOid": "2be50889e44faa2dbcf7547fab562442711b6cc5",
  "mergeBaseOid": "39544fd1c88147d33653d7d414c4e4b746a8a955",
  "lastPushDate": "2023-11-05T19:46:46.000Z",
  "lastActivityDate": "2023-11-17T01:10:05.000Z",
  "hasMergeConflict": true,
  "isFirstContribution": false,
  "tooManyFiles": false,
  "hugeChange": false,
  "popularityLevel": "Popular",
  "pkgInfo": [
    {
      "name": "react-relay",
      "kind": "edit",
      "files": [
        {
          "path": "types/react-relay/hooks.d.ts",
          "kind": "definition"
        },
        {
          "path": "types/react-relay/relay-hooks/EntryPointTypes.d.ts",
          "kind": "definition"
        },
        {
          "path": "types/react-relay/relay-hooks/loadQuery.d.ts",
          "kind": "definition"
        },
        {
          "path": "types/react-relay/relay-hooks/useQueryLoader.d.ts",
          "kind": "definition"
        },
        {
          "path": "types/react-relay/v7/lib/relay-experimental/EntryPointTypes.d.ts",
          "kind": "definition"
        },
        {
          "path": "types/react-relay/v7/lib/relay-experimental/loadQuery.d.ts",
          "kind": "definition"
        },
        {
          "path": "types/react-relay/v7/lib/relay-experimental/useQueryLoader.d.ts",
          "kind": "definition"
        }
      ],
      "owners": [
        "alloy",
        "maraisr",
        "edvinerikson",
        "levibuzolic"
      ],
      "addedOwners": [],
      "deletedOwners": [],
      "popularityLevel": "Popular"
    },
    {
      "name": "relay-runtime",
      "kind": "edit",
      "files": [
        {
          "path": "types/relay-runtime/index.d.ts",
          "kind": "definition"
        },
        {
          "path": "types/relay-runtime/lib/util/RelayConcreteNode.d.ts",
          "kind": "definition"
        },
        {
          "path": "types/relay-runtime/relay-runtime-tests.tsx",
          "kind": "test"
        }
      ],
      "owners": [
        "alloy",
        "maraisr",
        "morrys",
        "levibuzolic"
      ],
      "addedOwners": [],
      "deletedOwners": [],
      "popularityLevel": "Popular"
    }
  ],
  "reviews": [
    {
      "type": "stale",
      "reviewer": "maraisr",
      "date": "2023-11-06T08:22:05.000Z",
      "abbrOid": "28d0f0c"
    }
  ],
  "mainBotCommentID": 1793829726,
  "ciResult": "pass"
}

@typescript-bot
Copy link
Contributor

🔔 @alloy @maraisr @edvinerikson @levibuzolic @morrys — 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 added the The CI failed When GH Actions fails label Nov 5, 2023
@typescript-bot typescript-bot moved this from Waiting for Code Reviews to Needs Author Action in New Pull Request Status Board Nov 5, 2023
@typescript-bot
Copy link
Contributor

@tobias-tengler The CI build failed! Please review the logs for more information.

Once you've pushed the fixes, the build will automatically re-run. Thanks!

Note: builds which are failing do not end up on the list of PRs for the DT maintainers to review.

@typescript-bot typescript-bot removed the The CI failed When GH Actions fails label Nov 5, 2023
@typescript-bot typescript-bot moved this from Needs Author Action to Waiting for Code Reviews in New Pull Request Status Board Nov 5, 2023
@typescript-bot typescript-bot added the The CI failed When GH Actions fails label Nov 5, 2023
@typescript-bot typescript-bot moved this from Waiting for Code Reviews to Needs Author Action in New Pull Request Status Board Nov 5, 2023
@typescript-bot
Copy link
Contributor

@tobias-tengler The CI build failed! Please review the logs for more information.

Once you've pushed the fixes, the build will automatically re-run. Thanks!

Note: builds which are failing do not end up on the list of PRs for the DT maintainers to review.

@typescript-bot typescript-bot removed the The CI failed When GH Actions fails label Nov 5, 2023
@typescript-bot typescript-bot moved this from Needs Author Action to Waiting for Code Reviews in New Pull Request Status Board Nov 5, 2023
@typescript-bot typescript-bot added the The CI failed When GH Actions fails label Nov 5, 2023
@typescript-bot typescript-bot moved this from Waiting for Code Reviews to Needs Author Action in New Pull Request Status Board Nov 5, 2023
@typescript-bot
Copy link
Contributor

@tobias-tengler The CI build failed! Please review the logs for more information.

Once you've pushed the fixes, the build will automatically re-run. Thanks!

Note: builds which are failing do not end up on the list of PRs for the DT maintainers to review.

@typescript-bot typescript-bot moved this from Waiting for Code Reviews to Needs Maintainer Review in New Pull Request Status Board Nov 5, 2023
Copy link
Contributor

@maraisr maraisr left a comment

Choose a reason for hiding this comment

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

Nice work mate, thank you so much! Left one comment

types/relay-runtime/lib/util/RelayConcreteNode.d.ts Outdated Show resolved Hide resolved
@typescript-bot typescript-bot moved this from Needs Maintainer Review to Waiting for Code Reviews in New Pull Request Status Board Nov 6, 2023
@typescript-bot typescript-bot moved this from Waiting for Code Reviews to Needs Maintainer Review in New Pull Request Status Board Nov 6, 2023
@typescript-bot
Copy link
Contributor

@maraisr 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?

Copy link
Contributor

@maraisr maraisr left a comment

Choose a reason for hiding this comment

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

Looks good to me ✨ thank you so much

@typescript-bot typescript-bot added Owner Approved A listed owner of this package signed off on the pull request. Has Merge Conflict This PR can't be merged because it has a merge conflict. The author needs to update it. labels Nov 6, 2023
@typescript-bot typescript-bot moved this from Needs Maintainer Review to Needs Author Action in New Pull Request Status Board Nov 7, 2023
@typescript-bot
Copy link
Contributor

@tobias-tengler Unfortunately, this pull request currently has a merge conflict 😥. Please update your PR branch to be up-to-date with respect to master. Have a nice day!

@typescript-bot typescript-bot removed Has Merge Conflict This PR can't be merged because it has a merge conflict. The author needs to update it. Owner Approved A listed owner of this package signed off on the pull request. labels Nov 7, 2023
@typescript-bot typescript-bot moved this from Needs Author Action to Waiting for Code Reviews in New Pull Request Status Board Nov 7, 2023
@typescript-bot
Copy link
Contributor

@maraisr 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 Nov 7, 2023
@typescript-bot
Copy link
Contributor

Re-ping @alloy, @maraisr, @edvinerikson, @levibuzolic, @morrys:

This PR has been out for over a week, yet I haven't seen any reviews.

Could someone please give it some attention? Thanks!

@typescript-bot typescript-bot added the Unreviewed No one showed up to review this PR, so it'll be reviewed by a DT maintainer. label Nov 17, 2023
@maraisr
Copy link
Contributor

maraisr commented Nov 17, 2023

Think we might wait for this to land on the upstream, before merging this.

@typescript-bot typescript-bot added Has Merge Conflict This PR can't be merged because it has a merge conflict. The author needs to update it. and removed Unreviewed No one showed up to review this PR, so it'll be reviewed by a DT maintainer. labels Nov 21, 2023
@typescript-bot typescript-bot moved this from Needs Maintainer Review to Needs Author Action in New Pull Request Status Board Nov 21, 2023
@typescript-bot
Copy link
Contributor

@tobias-tengler Unfortunately, this pull request currently has a merge conflict 😥. Please update your PR branch to be up-to-date with respect to master. Have a nice day!

@typescript-bot typescript-bot added the Abandoned This PR had no activity for a long time, and is considered abandoned label Dec 10, 2023
@typescript-bot
Copy link
Contributor

@tobias-tengler I haven't seen any activity on this PR in more than three weeks, and it still has problems that prevent it from being merged. The PR will be closed on Dec 17th (in a week) if the issues aren't addressed.

@typescript-bot typescript-bot removed this from Needs Author Action in New Pull Request Status Board Dec 18, 2023
@typescript-bot
Copy link
Contributor

@tobias-tengler To keep things tidy, we have to close PRs that aren't mergeable and don't have activity in the last month. No worries, though — please open a new PR if you'd like to continue with this change. Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Abandoned This PR had no activity for a long time, and is considered abandoned Edits multiple packages Has Merge Conflict This PR can't be merged because it has a merge conflict. The author needs to update it. Popular package This PR affects a popular package (as counted by NPM download counts).
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants