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

Add support for esmodules for provided variables #3970

Closed
wants to merge 1 commit into from

Conversation

bigfootjon
Copy link
Member

@bigfootjon bigfootjon commented Jun 11, 2022

Fixes: #3966

I based my implementation on 0c4108d and efd9d86

Not great that we're duplicating the code for this around the codebase, happy to clean things up if anyone has any ideas.

Test plan:

??? I'm not really sure how to use this for my personal project locally and the test suite doesn't have any existing tests and my knowledge of Jest is... lacking.

@captbaritone
Copy link
Contributor

Thanks!

@facebook-github-bot
Copy link
Contributor

@captbaritone has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@mofeiZ
Copy link
Contributor

mofeiZ commented Jun 14, 2022

Thanks for trying out provided variables + helping us making this more usable for open source!

I think we'll also need to add type generation changes in the relay-typegen crate. Relay generate types for every provided variable used by a query to ensure that the provider module (1) exists and (2) exports a function of the correct type.

  • The code lives insrc/write.rs ->generate_provided_variables_type

The current TypeScript (+ flow) type-generation would produce types that look like this:

// graphql`
//  fragment Foo @argumentDefinitions(x : {type: "Float", provider: "getValue"}) {
//    my_field(arg: $x)
//  }
type ProvidedVariablesType = {
  readonly __relay_internal__pv__getValue: {
    readonly get: () => number | null;
  };
};

const providedVariablesDefinition: ProvidedVariablesType = {
  "__relay_internal__pv__getValue": require('./getValue')
};

@captbaritone
Copy link
Contributor

@bigfootjon are you still interested in working on this? If not we can close it.

@bigfootjon
Copy link
Member Author

Yeah sorry I don’t have the bandwidth to finish this :(

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.

[v14] Provided variables docs are wrong, or doesn't support esmodules
4 participants