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

Passing a Module reference to module throws Error #2476

Open
justin-equi opened this issue Dec 2, 2023 · 1 comment
Open

Passing a Module reference to module throws Error #2476

justin-equi opened this issue Dec 2, 2023 · 1 comment

Comments

@justin-equi
Copy link

justin-equi commented Dec 2, 2023

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
Copy link

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 }));

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

No branches or pull requests

2 participants