search

jaydenseric / graphql-upload

Middleware and an Upload scalar 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

CommonJS support #360

Closed tomcatmwi closed 1 year ago

tomcatmwi commented 1 year ago

Most Typescript projects are using module: "CommonJS". Having to refactor everything to switch to NodeNext is quite a challenge. I'm sure it would be easier to make graphql-upload usable in CommonJS projects without major changes.

jaydenseric commented 1 year ago

By far the easiest way forward for humanity in regards to minimising dev hours and improving outcomes (new technical possibilities like no build steps or bundling e.g. like Ruck) is for package authors to only publish standard ECMAScript modules and for project authors to migrate to the same, gradually phasing out their non standard CJS dependencies (an ESM Node.js project can import from both ESM and CJS dependencies). There is a huge amount of discussion in past issues where I explain all that, but a jumping in point for why not make graphql-upload a dual ESM/CJS package is here:

https://gist.github.com/sindresorhus/a39789f98801d908bbc7ff3ecc99d99c?permalink_comment_id=3850849#gistcomment-3850849

As a side note, you should be concerned if TypeScript NodeNext mode is a challenge in your project; it just reflects the realities of the modern Node.js runtime which presumably you are using. Not using NodeNext is just telling your dev tooling to ignore how your runtime and package ecosystem works.

Not migrating your projects to ESM accumulates organisational risk as it paints you into a CJS corner where over time you find yourself locked out of more and more ESM packages; some might be critical to what you need to build. At work recently while building an API that is a pure ESM codebase I needed to detect the MIME type of file streams, and could easily use the pure ESM dependency file-type to do it. If my project was CJS who knows how much time I would have wasted looking for an alternative CJS dependency. Eventually all maintained packages will be pure ESM.

tomcatmwi commented 1 year ago

You're not wrong, but if you really wish to discuss best practices, maybe let's start with how enabling multipart/form-data posts on a GraphQL API is a security risk.

I'm aware of the advantages of NodeNext, ESNext and other newer JS flavors. However, I don't think it's a good idea to try to force others to refactor entire projects. If the price is having to refactor the entire project, then maybe it's easier to just use some other solution. Or if that's not possible, convert it with Babel. On the other hand, if you really want to push a particular JS flavor, maybe it should be Typescript.

sirber commented 1 year ago

With TypeScript, it's easy to compile a CommonJS version and a ESM one, and publish the package with both.

https://www.sensedeep.com/blog/posts/2021/how-to-create-single-source-npm-module.html