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

Non string identifying argument values #895

Conversation

josephsavona
Copy link
Contributor

Note: this is inspired by and partially based on @iamchenxin's work in #767 and #844. Thanks for the head start!

Relay currently assumes that identifying argument values are strings (numbers sort of work, but not really). This builds on #894 (which added support to the plugin for parsing/printing literal InputObjects) by allowing identifying arguments to be basically anything - boolean, number, string, or array/object of the the same.

Key changes include:

  • Change the CallValue type from mixed to an explicit list of the supported types
  • Change forEachRootCallArg to return both the literal JS value of the argument as well as a serialized key
  • Change all callers of forEachRootCallArg (and some places that manually inspected the identifying arg) to correctly choose between the identifying argument value (i.e. when constructing a query with it) or the identifying argument key (for use with RelayRecordStore.{putDataID,getDataID}).
  • Added tests that the writer correctly creates root records for queries with non-string identifying arguments.

@josephsavona
Copy link
Contributor Author

@facebook-github-bot import

@facebook-github-bot
Copy link
Contributor

Thanks for importing. If you are an FB employee go to Phabricator to review.

@ghost ghost closed this in 38beb23 Mar 3, 2016
venepe pushed a commit to venepe/relay that referenced this pull request Mar 7, 2016
Summary:Note: this is inspired by and partially based on iamchenxin's work in facebook#767 and facebook#844. Thanks for the head start!

Relay currently assumes that identifying argument values are strings (numbers *sort* of work, but not really). This builds on facebook#894 (which added support to the plugin for parsing/printing literal InputObjects) by allowing identifying arguments to be basically anything - boolean, number, string, or array/object of the the same.

Key changes include:
- Change the `CallValue` type from mixed to an explicit list of the supported types
- Change `forEachRootCallArg` to return both the literal JS value of the argument as well as a serialized key
- Change all callers of `forEachRootCallArg` (and some places that manually inspected the identifying arg) to correctly choose between the identifying argument value (i.e. when constructing a query with it) or the identifying argument key (for use with `RelayRecordStore.{putDataID,getDataID}`).
- Added tests that the writer correctly creates root records with numeric/object identifying argument values.

Closes facebook#895

Reviewed By: yungsters

Differential Revision: D3003201

Pulled By: josephsavona

fb-gh-sync-id: 43ffbbd37e8d2bd8c0abe2cb792ad8959efb7a42
shipit-source-id: 43ffbbd37e8d2bd8c0abe2cb792ad8959efb7a42
@Visva92 Visva92 mentioned this pull request Mar 8, 2021
Closed
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants