Passing a `Module` reference to module throws Error
justin-equi opened this issue · 1 comments
justin-equi commented
Describe the bug
Some build systems do not expose prototype on Module, so when a caller passes a direct module interface into the module it blows up. I believe this is the actual correct behaviour for the Module object. I ran into this issue with a vitest setup. Seems unlikely this is the only situation where this might be an issue.
Example
//resolvers.ts
export const Mutation = {
yourMutation(){
... do things.
}
}And then the caller sets up the module as following
import { createModule } from 'graphql-modules';
import TypeDefs from './schema.graphql';
import * as Resolvers from './resolvers';
export const YourResolverModule = createModule({
id: 'your-resolver-module',
dirname: __dirname,
typeDefs: TypeDefs,
resolvers: Resolvers,
});Will fail with
TypeError: currentResolvers.hasOwnProperty is not a function
at mergeResolvers (file:///.node_modules/graphql-modules/index.mjs:1694:34)
at createResolvers (file://./node_modules/graphql-modules/index.mjs:1584:43)
at Object.factory (file:///./node_modules/graphql-modules/index.mjs:1971:40)
at file:///./node_modules/graphql-modules/index.mjs:1273:57
Fixes # (issue)
To Reproduce
Steps to reproduce the behavior:
//here is a repo that reproduces
https://github.com/jspears/graphql-modules-module-bug
Expected behavior
To not throw error
Environment:
- OS: Darwin Kernel Version 23.1.0: Mon Oct 9 21:28:45 PDT 2023; root:xnu-10002.41.9~6/RELEASE_ARM64_T6020
@graphql-modules/graphql-modules:- NodeJS:18.9.1
Additional context
mattjbrill commented
I ran into this one as well. Thank you for the fix, @justin-equi! Works great.
My environment has node 22.11.0, package.json "type": "module",, using typescript 5.6.3, tsconfig compilerOptions lib and target are ES2023 and module: ESNext.
My reproduction is similar to what's posted above. Difference is I'm passing arrays to typeDefs and resolvers in the createModule function.
If anyone else winds up here before the fix is merged, here's the generated patch-package file I'm using. I don't know if it's exactly what Justin did there, but it got rid of the error for me, and everything seems to work fine.... so far :)
file patches/graphql-modules+2.4.0.patch
diff --git a/node_modules/graphql-modules/index.js b/node_modules/graphql-modules/index.js
index a2789c5..83e499f 100644
--- a/node_modules/graphql-modules/index.js
+++ b/node_modules/graphql-modules/index.js
@@ -8,6 +8,9 @@ const wrap = require('@graphql-tools/wrap');
const ramda = require('ramda');
const ERROR_ORIGINAL_ERROR = 'diOriginalError';
+function hasOwnProperty(v, key){
+ return Object.prototype.hasOwnProperty.call(v, key);
+}
function getOriginalError(error) {
return error[ERROR_ORIGINAL_ERROR];
}
@@ -258,7 +261,7 @@ function forwardRef(forwardRefFn) {
}
function resolveForwardRef(type) {
if (typeof type === 'function' &&
- type.hasOwnProperty(forwardRefSymbol) &&
+ hasOwnProperty(type, forwardRefSymbol) &&
type[forwardRefSymbol] === forwardRef) {
return type();
}
@@ -1455,13 +1458,13 @@ function pickMiddlewares(path, middlewareMap) {
function validateMiddlewareMap(middlewareMap, metadata) {
const exists = checkExistence(metadata);
for (const typeName in middlewareMap.types) {
- if (middlewareMap.types.hasOwnProperty(typeName)) {
+ if (hasOwnProperty(middlewareMap.types, typeName)) {
const typeMiddlewareMap = middlewareMap[typeName];
if (!exists.type(typeName)) {
throw new ExtraMiddlewareError(`Cannot apply a middleware to non existing "${typeName}" type`, useLocation({ dirname: metadata.dirname, id: metadata.id }));
}
for (const fieldName in typeMiddlewareMap[typeName]) {
- if (typeMiddlewareMap[typeName].hasOwnProperty(fieldName)) {
+ if (hasOwnProperty(typeMiddlewareMap[typeName], fieldName)) {
if (!exists.field(typeName, fieldName)) {
throw new ExtraMiddlewareError(`Cannot apply a middleware to non existing "${typeName}.${fieldName}" type.field`, useLocation({ dirname: metadata.dirname, id: metadata.id }));
}
@@ -1498,7 +1501,7 @@ function createResolvers(config, metadata, app) {
const resolvers = addDefaultResolvers(mergeResolvers(config), middlewareMap, config);
// Wrap resolvers
for (const typeName in resolvers) {
- if (resolvers.hasOwnProperty(typeName)) {
+ if (hasOwnProperty(resolvers, typeName)) {
const obj = resolvers[typeName];
if (isScalarResolver(obj)) {
continue;
@@ -1508,7 +1511,7 @@ function createResolvers(config, metadata, app) {
}
else if (obj && typeof obj === 'object') {
for (const fieldName in obj) {
- if (obj.hasOwnProperty(fieldName)) {
+ if (hasOwnProperty(obj, fieldName)) {
ensure.type(typeName, fieldName);
const path = [typeName, fieldName];
// function
@@ -1609,7 +1612,7 @@ function mergeResolvers(config) {
const container = {};
for (const currentResolvers of resolvers) {
for (const typeName in currentResolvers) {
- if (currentResolvers.hasOwnProperty(typeName)) {
+ if (hasOwnProperty(currentResolvers, typeName)) {
const value = currentResolvers[typeName];
if (isNil(value)) {
continue;
@@ -1636,7 +1639,7 @@ function addObject({ typeName, fields, container, config, }) {
container[typeName] = {};
}
for (const fieldName in fields) {
- if (fields.hasOwnProperty(fieldName)) {
+ if (hasOwnProperty(fields, fieldName)) {
const resolver = fields[fieldName];
if (isResolveFn(resolver)) {
if (container[typeName][fieldName]) {
@@ -1683,7 +1686,7 @@ function addEnum({ typeName, resolver, container, config, }) {
container[typeName] = {};
}
for (const key in resolver) {
- if (resolver.hasOwnProperty(key)) {
+ if (hasOwnProperty(resolver, key)) {
const value = resolver[key];
if (container[typeName][key]) {
throw new ResolverDuplicatedError(`Duplicated resolver of "${typeName}.${key}" enum value`, useLocation({ dirname: config.dirname, id: config.id }));
diff --git a/node_modules/graphql-modules/index.mjs b/node_modules/graphql-modules/index.mjs
index f0ad8ce..eec48c8 100644
--- a/node_modules/graphql-modules/index.mjs
+++ b/node_modules/graphql-modules/index.mjs
@@ -4,6 +4,9 @@ import { wrapSchema } from '@graphql-tools/wrap';
import { mergeDeepWith } from 'ramda';
const ERROR_ORIGINAL_ERROR = 'diOriginalError';
+function hasOwnProperty(v, key){
+ return Object.prototype.hasOwnProperty.call(v, key);
+}
function getOriginalError(error) {
return error[ERROR_ORIGINAL_ERROR];
}
@@ -255,7 +258,7 @@ function forwardRef(forwardRefFn) {
}
function resolveForwardRef(type) {
if (typeof type === 'function' &&
- type.hasOwnProperty(forwardRefSymbol) &&
+ hasOwnProperty(type, forwardRefSymbol) &&
type[forwardRefSymbol] === forwardRef) {
return type();
}
@@ -1452,13 +1455,13 @@ function pickMiddlewares(path, middlewareMap) {
function validateMiddlewareMap(middlewareMap, metadata) {
const exists = checkExistence(metadata);
for (const typeName in middlewareMap.types) {
- if (middlewareMap.types.hasOwnProperty(typeName)) {
+ if (hasOwnProperty(middlewareMap.types, typeName)) {
const typeMiddlewareMap = middlewareMap[typeName];
if (!exists.type(typeName)) {
throw new ExtraMiddlewareError(`Cannot apply a middleware to non existing "${typeName}" type`, useLocation({ dirname: metadata.dirname, id: metadata.id }));
}
for (const fieldName in typeMiddlewareMap[typeName]) {
- if (typeMiddlewareMap[typeName].hasOwnProperty(fieldName)) {
+ if (hasOwnProperty(typeMiddlewareMap[typeName], fieldName)) {
if (!exists.field(typeName, fieldName)) {
throw new ExtraMiddlewareError(`Cannot apply a middleware to non existing "${typeName}.${fieldName}" type.field`, useLocation({ dirname: metadata.dirname, id: metadata.id }));
}
@@ -1495,7 +1498,7 @@ function createResolvers(config, metadata, app) {
const resolvers = addDefaultResolvers(mergeResolvers(config), middlewareMap, config);
// Wrap resolvers
for (const typeName in resolvers) {
- if (resolvers.hasOwnProperty(typeName)) {
+ if (hasOwnProperty(resolvers, typeName)) {
const obj = resolvers[typeName];
if (isScalarResolver(obj)) {
continue;
@@ -1505,7 +1508,7 @@ function createResolvers(config, metadata, app) {
}
else if (obj && typeof obj === 'object') {
for (const fieldName in obj) {
- if (obj.hasOwnProperty(fieldName)) {
+ if (hasOwnProperty(obj, fieldName)) {
ensure.type(typeName, fieldName);
const path = [typeName, fieldName];
// function
@@ -1606,7 +1609,7 @@ function mergeResolvers(config) {
const container = {};
for (const currentResolvers of resolvers) {
for (const typeName in currentResolvers) {
- if (currentResolvers.hasOwnProperty(typeName)) {
+ if (hasOwnProperty(currentResolvers, typeName)) {
const value = currentResolvers[typeName];
if (isNil(value)) {
continue;
@@ -1633,7 +1636,7 @@ function addObject({ typeName, fields, container, config, }) {
container[typeName] = {};
}
for (const fieldName in fields) {
- if (fields.hasOwnProperty(fieldName)) {
+ if (hasOwnProperty(fields, fieldName)) {
const resolver = fields[fieldName];
if (isResolveFn(resolver)) {
if (container[typeName][fieldName]) {
@@ -1680,7 +1683,7 @@ function addEnum({ typeName, resolver, container, config, }) {
container[typeName] = {};
}
for (const key in resolver) {
- if (resolver.hasOwnProperty(key)) {
+ if (hasOwnProperty(resolver, key)) {
const value = resolver[key];
if (container[typeName][key]) {
throw new ResolverDuplicatedError(`Duplicated resolver of "${typeName}.${key}" enum value`, useLocation({ dirname: config.dirname, id: config.id }));