graphile / crystal-pre-merge

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

Improve errors; forbid a bad pattern #478

Closed benjie closed 11 months ago

benjie commented 11 months ago

Calling getTypeByName directly or indirectly inside a type generator function is not allowed:

// BAD!
registerObjectType(
  'Foo',
  {},
  () => {
    const Bar = getTypeByName('Bar');
    return {
      fields: {
        bar: {
          type: Bar
        }
      }
    }
  },
  ''
);

it can only be called from within callbacks that that spec contains:

// Fine
registerObjectType(
  'Foo',
  {},
  () => {
    return {
      fields() {
        const Bar = getTypeByName('Bar');
        return {
          bar: {
            type: Bar
          }
        };
      }
    }
  },
  ''
);

The reason for this is it has the potential to trigger hard to debug and hard to reproduce cycles.

This PR enforces this rule by throwing if getTypeByName is called again when getTypeByName is currently running. This found the problem in 5 places in the core codebase, so I wouldn't be surprised if it also found issues with plugins. However, it's a net win because it should completely eradicate this class of error.

Thanks to @andreyobrezkov for bringing this to my attention.

benjie commented 11 months ago

When reviewing this PR, be sure to turn whitespace off, not as much has changed as it appears!