graphile / crystal-pre-merge

Repository moved
https://github.com/graphile/crystal
39 stars 10 forks source link

graphile-export optimization: inline objects in fields() #424

Open benjie opened 11 months ago

benjie commented 11 months ago

Currently Graphile Export attempts to make your code as efficient as possible via hoisting - anything that can be hoisted to the top will be. We should selectively un-hoist some of these things for legibility, especially where the hoisting confers no benefit.

One example is the GraphQLObjectTypeSpec.fields() callback. It will only be called once, so there's no need to hoist. i.e.

const SimilarTable1_col2_extensions = Object.assign(Object.create(null), {
  grafast: {
    plan($record) {
      return $record.get("col2");
    }
  }
});

export const SimilarTable1 = new GraphQLObjectType({
  name: "SimilarTable1",
  fields() {
    return {
      col2: {
        type: GraphQLInt,
        extensions: SimilarTable1_col2_extensions
      },
    };
  }
});

should become:

export const SimilarTable1 = new GraphQLObjectType({
  name: "SimilarTable1",
  fields() {
    return {
      col2: {
        type: GraphQLInt,
        extensions: Object.assign(Object.create(null), {
          grafast: {
            plan($record) {
              return $record.get("col2");
            }
          }
        })
      },
    };
  }
});

We should also consider selectively downgrading Object.assign(Object.create(null), { ... }) to just { ... } where it's safe to do so. Maybe via a configuration flag ("dangerouslyReadable" 😉 )