jaydenseric / graphql-upload

Middleware and a scalar Upload to add support for GraphQL multipart requests (file uploads via queries and mutations) to various Node.js GraphQL servers.
https://npm.im/graphql-upload
MIT License
1.43k stars 132 forks source link

Uploading file crashes application when built with webpack #258

Closed ziggy6792 closed 3 years ago

ziggy6792 commented 3 years ago

Hi

I am trying to use graphql-upload in a typescript app built with webpack.

My setup

My Code

(stripped down to the essentials)

https://github.com/ziggy6792/graphql-file-uploads

My Problem

When I run with ts-node yarn start:ts:node then run yarn test an image gets uploaded no problem

But then I build with webpack yarn start then run yarn test I get the following error

/Users/sive/Documents/workspace/graphql-file-uploads/dist/index.js:56105
            if (!isObject(operations) && !Array.isArray(operations))
                 ^

TypeError: isObject is not a function
    at Busboy.<anonymous> (/Users/sive/Documents/workspace/graphql-file-uploads/dist/index.js:56105:18)
    at Busboy.emit (node:events:378:20)
    at Busboy.module.exports../node_modules/busboy/lib/main.js.Busboy.emit (/Users/sive/Documents/workspace/graphql-file-uploads/dist/index.js:32204:33)
    at PartStream.onEnd (/Users/sive/Documents/workspace/graphql-file-uploads/dist/index.js:32528:15)
    at PartStream.emit (node:events:390:22)
    at Dicer.onPart (/Users/sive/Documents/workspace/graphql-file-uploads/dist/index.js:32386:13)
    at Dicer.emit (node:events:378:20)
    at Dicer.module.exports../node_modules/dicer/lib/Dicer.js.Dicer.emit (/Users/sive/Documents/workspace/graphql-file-uploads/dist/index.js:45439:35)
    at Dicer.module.exports../node_modules/dicer/lib/Dicer.js.Dicer._oninfo (/Users/sive/Documents/workspace/graphql-file-uploads/dist/index.js:45540:12)
    at SBMH.<anonymous> (/Users/sive/Documents/workspace/graphql-file-uploads/dist/index.js:45486:10)
[nodemon] app crashed - waiting for file changes before starting...

My Attempts to solve

Sorry I am not sure if this actually a problem with graphql-upload or webpack or something else. I also noticed that my issue looks similar to this issue, but I didn't really understand if there were any fixes to that issue and if indeed it is the same issue I am facing.

I tried to upgrade webpack (branch upgraded-packages) but this made the problem worse as I get this issue when I start the server

TypeError: Cannot read property 'graphql' of undefined
    at getPeerDependencyGraphQLRequirement (/Users/sive/Documents/workspace/graphql-file-uploads/dist/index.js:104216:44)
    at Object.ensureInstalledCorrectGraphQLPackage (/Users/sive/Documents/workspace/graphql-file-uploads/dist/index.js:104221:32)
    at Function.checkForErrors (/Users/sive/Documents/workspace/graphql-file-uploads/dist/index.js:103346:27)
    at Function.generateFromMetadataSync (/Users/sive/Documents/workspace/graphql-file-uploads/dist/index.js:103325:14)
    at Function.generateFromMetadata (/Users/sive/Documents/workspace/graphql-file-uploads/dist/index.js:103315:29)
    at Object.buildSchema (/Users/sive/Documents/workspace/graphql-file-uploads/dist/index.js:103940:61)
    at /Users/sive/Documents/workspace/graphql-file-uploads/dist/index.js:99831:41
    at Generator.next (<anonymous>)
    at /Users/sive/Documents/workspace/graphql-file-uploads/dist/index.js:99819:71
    at new Promise (<anonymous>)

Any help would be greatly appreciated.

jaydenseric commented 3 years ago

But then I build with webpack yarn start then run yarn test I get the following error

I cloned your repo, but there is no yarn test script…

ziggy6792 commented 3 years ago

Wow you replied fast.

Sorry forgot to push Please pull again :)

jaydenseric commented 3 years ago

Your problem is to do with your project's webpack config; it's not a bug with graphql-upload.

When webpack tries to bundle this:

https://github.com/jaydenseric/graphql-upload/blob/60f428bafd85b93bc36524d1893aa39501c50da1/public/processRequest.js#L6

Instead of correctly resolving the CJS file that Node.js would (using the main field):

https://github.com/jonschlinkert/isobject/blob/15d5d58ea9fbc632dffd52917ac6791cd92251ab/package.json#L19

It tries to resolve a faux ESM file using the non standard module field:

https://github.com/jonschlinkert/isobject/blob/15d5d58ea9fbc632dffd52917ac6791cd92251ab/package.json#L20

In Node.js (and just generally speaking due to the concepts), you can import CJS into ESM, but you can't require ESM into CJS. In this situation webpack is doing some tricky non-standard things to try to get it to work. Most likely it's creating an extra default property in the faux CJS representation it creates, and the function that's undefined in your runtime error would probably be available in that context as isObject.default.

As an experiment, if you update the webpack config to add mainFields: ['main'] under resolve, you'll see that with a fresh yarn build and yarn start, the yarn test script works without an error.

Note that bundlers like webpack and rollup came up with the module field to try to get tree-shaking to work, so be careful if removing it bloats your bundle size.

You could look into the config for webpack and maybe even Babel or TypeScript (didn't look too closely at your setup), whichever is applicable, regarding ESM/CJS default interop so that the transpilation or bundling will wrap the require with a helper that accounts for the .default.

Stepping back though, as a general rule if Node.js can't run and resolve it, don't do it. Don't generate or consume faux ESM incompatible with Node.js, avoid all the legacy bundler magic (e.g. module field) and update your tools as soon as you can and take advantage of modern Node.js support for ESM, package exports fields, etc.

Webpack v5 is way better than the v4 you're using in that reproduction repo, so before fiddling around with webpack config to get things to work update webpack or your work will be wasted when you do upgrade.

Assuming you really do need to bundle, have you considered using esbuild instead? It's way faster and easier to use than webpack.

ziggy6792 commented 3 years ago

Hi @jaydenseric

Thanks so much for such a detailed answer.

Firstly, your experimental fix (mainFields: ['main']) for using webpack 4 works! However, I get the strong impression from you that I should not be satisfied with that solution and it may lead to problems down the line.

Based on your recommendations, I tried the following setups;

TypeError: Cannot read property 'graphql' of undefined
    at getPeerDependencyGraphQLRequirement (/Users/sive/Documents/workspace/graphql-file-uploads/dist/index.js:104216:44)
    at Object.ensureInstalledCorrectGraphQLPackage (/Users/sive/Documents/workspace/graphql-file-uploads/dist/index.js:104221:32)
    at Function.checkForErrors (/Users/sive/Documents/workspace/graphql-file-uploads/dist/index.js:103346:27)
    at Function.generateFromMetadataSync (/Users/sive/Documents/workspace/graphql-file-uploads/dist/index.js:103325:14)
    at Function.generateFromMetadata (/Users/sive/Documents/workspace/graphql-file-uploads/dist/index.js:103315:29)
    at Object.buildSchema (/Users/sive/Documents/workspace/graphql-file-uploads/dist/index.js:103940:61)
    at /Users/sive/Documents/workspace/graphql-file-uploads/dist/index.js:99831:41
    at Generator.next (<anonymous>)
    at /Users/sive/Documents/workspace/graphql-file-uploads/dist/index.js:99819:71
    at new Promise (<anonymous>)
NoExplicitTypeError: Unable to infer GraphQL type from TypeScript reflection system. You need to provide explicit type for 'firstName' of 'User' class.
    at Object.findType (webpack://type-graphql-series/./node_modules/type-graphql/dist/helpers/findType.js?:19:15)
    at eval (webpack://type-graphql-series/./node_modules/type-graphql/dist/decorators/Field.js?:16:53)
    at __decorateClass (webpack://type-graphql-series/./src/entity/User.ts?:13:24)
    at eval (webpack://type-graphql-series/./src/entity/User.ts?:24:1)
    at Module../src/entity/User.ts (/Users/sive/Documents/workspace/graphql-file-uploads/dist/index.js:3399:1)
    at __webpack_require__ (/Users/sive/Documents/workspace/graphql-file-uploads/dist/index.js:10636:42)
    at eval (webpack://type-graphql-series/./src/modules/user/Register.ts?:7:70)
    at Module../src/modules/user/Register.ts (/Users/sive/Documents/workspace/graphql-file-uploads/dist/index.js:3432:1)
    at __webpack_require__ (/Users/sive/Documents/workspace/graphql-file-uploads/dist/index.js:10636:42)
    at eval (webpack://type-graphql-series/./src/index.ts?:11:80)
[nodemon] app crashed - waiting for file changes before starting...
Same error as above

If you could provide anymore tips on getting webpack 5 or esbuild working that would be amazing. However I'm am sure these errors have got nothing to do with graphql-upload and I know you have better things to do than teach me how to use bundlers (and this not the right place this kind of support).

By the way the only reason I want to bundle in the first place is because I want to use

Thanks so much for your support. You clearly know your stuff.

ziggy6792 commented 3 years ago

Hey @jaydenseric

I worked it out with esbuild I just needed to use an extra plugin @anatine/esbuild-decorators

Thanks again for your help.

rickklaasboer commented 3 years ago

If anyone comes across this and wishes to still use Webpack (5). The fix is relatively simple. You could just alias both imports for graphql/isobject.

alias: {
    graphql$: path.resolve(
        __dirname,
        './node_modules/graphql/index.js',
    ),
    isobject$: path.resolve(
        __dirname,
        './node_modules/isobject/index.cjs.js',
    ),
},
ziggy6792 commented 3 years ago

Hi @rickklaasboer

Thanks for your input. I would really like to get webpack5 working.

I tried your suggestion and pushed to branch webpack5

resolve: {
      extensions: ['.mjs', '.ts', '.js'],
      alias: {
        graphql$: path.resolve(__dirname, './node_modules/graphql/index.js'),
        isobject$: path.resolve(__dirname, './node_modules/isobject/index.cjs.js'),
      },
      plugins: [
        new TsconfigPathsPlugin({
          /* options: see below */
        }),
      ],
    },

However I still get the same error when I start

 TypeError: Cannot read property 'graphql' of undefined
    at getPeerDependencyGraphQLRequirement (/Users/sive/Documents/workspace/graphql-file-uploads/dist/index.js:115757:44)
    at Object.ensureInstalledCorrectGraphQLPackage (/Users/sive/Documents/workspace/graphql-file-uploads/dist/index.js:115762:32)
    at Function.checkForErrors (/Users/sive/Documents/workspace/graphql-file-uploads/dist/index.js:114887:27)
    at Function.generateFromMetadataSync (/Users/sive/Documents/workspace/graphql-file-uploads/dist/index.js:114866:14)
    at Function.generateFromMetadata (/Users/sive/Documents/workspace/graphql-file-uploads/dist/index.js:114856:29)
    at Object.buildSchema (/Users/sive/Documents/workspace/graphql-file-uploads/dist/index.js:115481:61)
    at /Users/sive/Documents/workspace/graphql-file-uploads/dist/index.js:111372:41
    at Generator.next (<anonymous>)
    at /Users/sive/Documents/workspace/graphql-file-uploads/dist/index.js:111360:71
    at new Promise (<anonymous>)

Thanks

rickklaasboer commented 3 years ago

I've checked out your webpack5 branch locally and it seems to be working just fine for me. Maybe try reinstalling your node_modules?

ziggy6792 commented 3 years ago

Hmm this is weird.

I even recloned the whole project and I still get the error. Is it a difference in node version maybe?

node -v
v15.11.0
yarn -v
1.22.10

I will add a docker build and try reproduce the problem with any variables of our system taken out.

Update: I have added docker build and run to my repo

Can run with docker:build and docker:run. It works with webpack 4 but not webpack 5