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

Update rescript relay fork #3

Merged
merged 348 commits into from
Aug 5, 2022
Merged

Conversation

zth
Copy link
Owner

@zth zth commented Aug 5, 2022

Updates and adapts the fork from commit facebook@cee80ff to commit 470a5fbc525f5dfc5a3a232f15d217420136b72c

captbaritone and others added 30 commits May 31, 2022 16:41
Reviewed By: davidmccabe

Differential Revision: D36782871

fbshipit-source-id: aa9cb152a72a0e023cefa9149be93418a1a6c62a
Reviewed By: davidmccabe

Differential Revision: D36784412

fbshipit-source-id: 089d396c71264642c0d065f90e8cd7453837f3a6
Reviewed By: davidmccabe

Differential Revision: D36785201

fbshipit-source-id: 692a910db8e930e5a74d9546cd9fcaaa2513c4a7
Reviewed By: davidmccabe

Differential Revision: D36788014

fbshipit-source-id: c2ade2460503d5ebfc785648be8556bced79a0a1
Reviewed By: ndmitchell

Differential Revision: D36811272

fbshipit-source-id: 7a9384d685b230c00ce250a37c91421f65c26b62
Summary:
Inspired by D36649352 (facebook@73f01ac) — useFragment() currently has to repeatedly check whether a given fragment result has missing client edges. In the new React Cache version of these hooks, this incurs an extra useEffect call. However, the majority of fragments won't have client edges — ideally we could completely avoid the runtime overhead for fragments that don't use this feature.

This diff changes the compiler to emit `hasClientEdges` metadata on fragments. A follow-up will change hooks to optimize based on this and avoid work for fragments that don't have any client edges.

Reviewed By: davidmccabe

Differential Revision: D36682923

fbshipit-source-id: b836640a9a6e716d5421ec61e8d57f1312505c84
Summary:
The compiler playground does not currently build due to the version of parkling_lot that we are using being incompatible with wasm-pack. I manually updated the parking_lot version to `0.12` in order to get it building again. This uncovered some tests which are currently failing. Updating these tests will make it easier to get the the compiler playground building again once we upgrade parking_lot officially.

Currently upgrading is somewhat complicated because we try to only have one version of each dependency installed at any given time in the Meta monorepo.

Pull Request resolved: facebook#3927

Reviewed By: josephsavona

Differential Revision: D36789186

Pulled By: captbaritone

fbshipit-source-id: 448502cdee3dd209c6f053e9cb236420e92dfc32
Reviewed By: davidmccabe

Differential Revision: D36792818

fbshipit-source-id: be04278b0a432b9ec1ad5096c2999407ed321766
Reviewed By: davidmccabe

Differential Revision: D36793042

fbshipit-source-id: a01fd49db01a63606edad87dddda44714acf1b44
Summary:
The goal of this PR is to support `no_inline` in CommonJS, but I did a little clean up along the way.

We used to clone the `no_inline` directive to all of the FragmentSpreads that used that source fragment. That felt like a hack so I switched it over to use `NoInlineMetadata` which uses the cool and very hip associated data stuff.

Pull Request resolved: facebook#3923

Reviewed By: josephsavona

Differential Revision: D36820179

Pulled By: captbaritone

fbshipit-source-id: e69449410f4e06b0b213a59894cee96342eca049
Summary: Uses the metadata added in the previous diff to avoid even looking for missing client edges for fragments that can never have them.

Reviewed By: davidmccabe

Differential Revision: D36684124

fbshipit-source-id: 7522b84676de6aefe9511827c0ec1b2539a22543
Summary:
Unrevert of D36492040 (facebook@b338119) which was reverted due to a Flow error (fixed/suppressed here) that for some reason didn't show up in Sandcastle or stop it from landing.

Allow the implementation of some hooks to be replaced -- to enable turning on via feature flag without increasing bundle size when off.

Reviewed By: josephsavona

Differential Revision: D36818964

fbshipit-source-id: 408797999dbfb1577a3842da7b7a71d24a8aa384
Reviewed By: captbaritone

Differential Revision: D36810021

fbshipit-source-id: 2fdc054e0e8c2f476fd6ff8f6819cb940b6781e2
…e query

Reviewed By: captbaritone

Differential Revision: D36810020

fbshipit-source-id: 441df8d6a850d894d27b9a23a403ab5b522c9e48
…ies/Relay

Reviewed By: alunyov

Differential Revision: D36820258

fbshipit-source-id: b168ca1dd46df51b822e30c8fac50ecdad538e96
…ies/Relay (tests)

Reviewed By: alunyov

Differential Revision: D36836715

fbshipit-source-id: 529cd9483c9376b009295ee3aeb450ab532b9163
Reviewed By: alunyov

Differential Revision: D36823188

fbshipit-source-id: 5f3c33bf1dff01b371737669daaae57bb14f93c1
Summary:
Previously, it was not possible to make test assertions about how our hover code bahaved. This set of fixture tests should allow us to validate our hover behavior.

This should also make it possible for us to collect an overview of our existing hover behavior and potentially make it more consistent.

Reviewed By: alunyov

Differential Revision: D36854204

fbshipit-source-id: 36b94aa2ca42393a779a5a2c06e4d2f08fb51f85
Reviewed By: gkz

Differential Revision: D36710466

fbshipit-source-id: 4dff0bf2f57d695abc183be9f89147f239fb4953
Summary:
`[ERROR] ✖︎ Variables are not yet supported inside inline fragments.`

This PR will enable query variables to be used inside of `inline` fragments. I will work to get `argument` style variables working in a follow up PR. I'll try to outline what work I think is required to support fragment variables.

## Query Variables
Easy to turn on, I wrote a test to confirm that this works as expected.

## Fragment Variables with `arguments`
Not so easy, I'll try to layout what I found while diving deep.

### How `useFragment` works today

When reading the parent operation definition, fragment pointers are created. If the reader AST for that fragment has an `args` field (this comes from `argumentDefinitions`), the translated variables are inlined in the fragment pointer object.

When the useFragment for the fragment with `argumentDefinitions` is used, the variables from the parent's pointer are used in conjunction with the query's variables.

### What needs to change?
It seems like `InlineDataFragmentSpread` should have an `args` field in the reader AST. If we had that, we could temporarily swap out `this._variables` with the translated variables for the duration of `_createInlineDataOrResolverFragmentPointer` using `getArgumentValues`.

Pull Request resolved: facebook#3933

Reviewed By: alunyov

Differential Revision: D36828783

Pulled By: captbaritone

fbshipit-source-id: a60e33114bf44577f4c5fbd60d382c1d9e160ea1
…braries/Relay (reapply)

Reviewed By: bradzacher

Differential Revision: D36875558

fbshipit-source-id: 822ebb808661cfbb2b385c1b15944259a16363da
Reviewed By: josephsavona

Differential Revision: D36362318

fbshipit-source-id: c3dfca2618efdf02133ad3d1223a79c34db86065
Reviewed By: zertosh

Differential Revision: D36881794

fbshipit-source-id: 44fe8805ed4588ea4c8092124543b9e117f22ba9
Summary:
This should allow the compiler explorer to be used as a mechanism for sharing minimal reproducible compiler bugs and other examples of compiler behavior.

The general idea is to consolidate all the state in the compiler explorer into a single object (Redux style):

* Schema
* Document
* Feature Flags
* Language
* Selected tab

And then (json) serialize and compress that state and persist it to the URL hash and local storage. Note that by using the URL hash we ensure the content of the explorer is never sent to the server, and thus the privacy of the content is improved.

Pull Request resolved: facebook#3930

Test Plan:
https://user-images.githubusercontent.com/162735/171328459-a675c8e5-a9a6-4a27-af2e-1230245b84d2.mov

**Static Docs Preview: relay**
|[Full Site](https://our.intern.facebook.com/intern/staticdocs/eph/1654205352/relay/)|

|**Modified Pages**|

**Static Docs Preview: relay**
|[Full Site](https://our.intern.facebook.com/intern/staticdocs/eph/D36845080/V3/relay/)|

|**Modified Pages**|

Reviewed By: alunyov

Differential Revision: D36845080

Pulled By: captbaritone

fbshipit-source-id: 532cdbed4e0b1014ff3232e1b8aa47dce16cafa1
Reviewed By: zertosh

Differential Revision: D36881793

fbshipit-source-id: ce9b86693dd18efa6884d29419970b41ebff5447
Reviewed By: alunyov

Differential Revision: D36886101

fbshipit-source-id: e5cef7c6c351a16de805dcfb3c3cb3f75d7f2a5e
Summary: Micro-optimization to avoid an unnecessary `useMemo` call in the common case. Previously the new useFragmentInternal_REACT_CACHE() hook memoized the translation from internal hook to fragment result. However, for singular fragments this is a simple property lookup and isn't worth memoizing. Here, we exploit the fact that plurality is a static property of the fragment (doesn't change dynamically for a particular fragment invocation) and only memoize the computation for plural fragments.

Reviewed By: davidmccabe

Differential Revision: D36796332

fbshipit-source-id: 5651e62f9c2502f1a634cc825719e3fe4ed7fefd
Reviewed By: rbalicki2

Differential Revision: D36905286

fbshipit-source-id: 5b5e70f03cb1d57fe488911ea668eb743f327f25
…g literal flowtype

Summary:
# What and why

* Currently, a selection like `foo { __typename }` will generate a type of `__typename: string`. We have never had a need to generate a more accurate type, hence we generate `string` instead of a string literal for the value of `__typename`.
* However, this is problematic for typesafe updaters! This requires assignments of `foo` to be validated, **even though we known statically that the validation can never fail**.
* With this change, assigning an array of linked fields is modified as follows:

```
const data = useFragment(graphql`fragment SourceFragment on User {
  best_friends(count: 5) required(action: THROW) {
    ...Assignable_user
  }
}`, userRef);

const env = useRelayEnvironment();
const onClick = () => {
  env.commitUpdate(store => {
    const {updatableData} = store.readUpdatableQuery(graphql`
      query Updatable_best_friends updatable {
        best_friends {
          ...Assignable_user
        }
      }
    `, {});

    // WE CAN REPLACE THIS
    const validateUser = require('SourceFragment').validate;
    const validBestFriends = data.best_friends.flatMap(bestFriend => {
      const validBestFriend = validateUser(bestFriend);
      if (validBestFriend !== false) {
        return [validBestFriend];
      } else {
        // this case never occurs!
        return [];
      }
    });
    updatableData.best_friends = validBestFriends;

    // WITH THIS
    updatableData.best_friends = data.best_friends
  });
}
```

# How

Pass around a `enclosing_concrete_linked_field_type: Option<Type>` through the codegen. Whenever we visit a linked field, we potentially pass `Some(linked_field_type)`. When we process a scalar field, if that parameter is Some, then we generate an AST with the StringLiteral of tha type.

# Rollout

This needs to rollout with a rollout. Once this diff is approved, I'll put a rollout in.

Reviewed By: alunyov

Differential Revision: D36357062

fbshipit-source-id: e06d076e6395183eb7872e47962c81e6cd4db475
Reviewed By: tyao1

Differential Revision: D36909200

fbshipit-source-id: cde71524c1ff4357ca15ee896f80e11e781fe6a8
voideanvalue and others added 27 commits August 2, 2022 08:05
Reviewed By: alunyov

Differential Revision: D38349024

fbshipit-source-id: 66beb5bcc94c0198e956a6ff867ecaa1e6bec5c4
Summary: Making 'normalization' entries while generating artifacts to be Optional.  This change is to support additional changes which will allow client-side updatable queries that have no server-side fields.

Reviewed By: rbalicki2

Differential Revision: D38020615

fbshipit-source-id: d00161d2bba6a297381a110fddcb20791be08cce
Summary: Adding a test to validate simple `updatable` queries with client-side only extensions and updating the `updatable` test to support.

Reviewed By: rbalicki2

Differential Revision: D38253485

fbshipit-source-id: 2802c92ebb685feb380441294376b40e19fab5a2
Summary:
Updatable queries do not get sent to the server. Consider usePrivacyIncidentFactGatheringContentStateQuery.graphql.js. The generated artifact only includes fragment and kind fields, i.e. no operation and no metadata. As a consequence, all of the work that we do to updatable queries in the `apply_operation_transforms`, `apply_normalization_transforms`, and `apply_operation_text_transforms` pipelines in apply_transforms.rs:

* is useless, since the results aren't used
* are actively harmful, in that the transforms in this pipeline enforce certain invariants that make no sense. e.g. updatable queries with only client only fields are disallowed — but that is in fact fine!

This change modifies the underpinnings to properly skip client-side-only `updatable` Queries.  The core problem of erroring out remains, since other logic causes empty queries to panic.

Reviewed By: rbalicki2

Differential Revision: D38132333

fbshipit-source-id: f1dea8f449ac9d5d289edaadcbdad18e1ee27987
Reviewed By: voideanvalue

Differential Revision: D38272361

fbshipit-source-id: 4f9086cbdf08601cc20ba54b20d8497e47ec39a8
Reviewed By: tyao1

Differential Revision: D38402782

fbshipit-source-id: 0da4d39d22ea20416a970fb0f17a7bd502777af4
Reviewed By: SamChou19815

Differential Revision: D38415143

fbshipit-source-id: f5c672dc7fca7e0e81a65963cbd148bfca5deaf3
Differential Revision: D38327619

fbshipit-source-id: c92b2ee1c80b3059c5f35031d5184abe4df1996e
Summary:
Pull Request resolved: facebook#4036

Adding this to both have a consistency among functions and for my internal use case.

We have `get_*` functions for other types but `unions` was missing. Adding `get_unions` function to have consistency and support relevant use cases.

Reviewed By: schaitoff

Differential Revision: D38387300

fbshipit-source-id: 18af71f8e282e87d489f4c5dfde1dc13cc9fd3ca
@zth zth merged commit 8041b58 into rescript-relay Aug 5, 2022
zth added a commit that referenced this pull request Aug 5, 2022
@zth zth deleted the update-rescript-relay-fork branch August 5, 2022 07:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.