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

Support node 17 & typescript #306

Closed
meabed opened this issue May 23, 2022 · 2 comments
Closed

Support node 17 & typescript #306

meabed opened this issue May 23, 2022 · 2 comments

Comments

@meabed
Copy link

meabed commented May 23, 2022

Thank you for making this 🙏

First issue:

In the last version you have add node18 support, but there is node 17 as well :)
It would be great to add this to the package json:

error [email protected]: The engine "node" is incompatible with this module. Expected version "^14.17.0 || ^16.0.0 || >= 18.0.0". Got "17.9.0"

Second issue:

The typescript "@types/graphql-upload": "8.0.11" is not incompatible with the package, it's going to very helpful if this is updated or published the types in the package it self.

Old version import 13

import { GraphQLUpload } from 'graphql-upload';
import { graphqlUploadExpress } from 'graphql-upload';
import { FileUpload } from "graphql-upload";

New version import 14

import GraphQLUpload from 'graphql-upload/GraphQLUpload.js';
import graphqlUploadExpress from 'graphql-upload/graphqlUploadExpress.js';
import { FileUpload } from "graphql-upload"; // not existing

@jaydenseric
Copy link
Owner

Thanks for the thanks, glad to help :)

First issue:

In the last version you have add node18 support, but there is node 17 as well :)

In just a week (2022-06-01), Node.js v17 reaches end-of-life:

https://nodejs.org/en/about/releases/

It's not supported for the same reason Node.js v15 isn't supported.

Even if we wanted to support it, pretty soon we can't anyway as dependencies and dev dependencies drop the end-of-life Node.js v17 from their support, for example graphql v17:

https://github.com/graphql/graphql-js/blob/c7d7026982ceee536900a24ae31235127560297a/package.json#L27-L29

By getting ahead of that in this major release we can later update dependencies that drop Node.js v17 more easily without having to publish a new major version or document it in the changelog entry under major breaking changes, which is distracting even if we needed to publish a major version for other reasons.

It seems like you are combining 2 things in your second point of feedback? Regarding @types/graphql-upload, you should uninstall that from your projects as it's redundant now types are published within graphql-upload. To make use of them properly, see these tips:

#282 (comment)

Regarding the package main index module being removed and import paths changing, that is on purpose and is documented in the graphql-upload v14 changelog entry under major changes. You can see an explanation for this change here:

#305 (comment)

The types can be imported (via TypeScript type imports, e.g. within JSDoc types) directly from the specific modules they originate from. For example:

https://github.com/jaydenseric/apollo-upload-examples/blob/86e5609e60d86a92fd6060679641105beec715e5/api/storeUpload.mjs#L10-L12

Note that the TypeScript types published in graphql-upload have been created from scratch by me, and are not intended to match exactly whatever types previously existed in @types/graphql-upload.

@meabed
Copy link
Author

meabed commented May 23, 2022

Thank you so much for the reply.

I got your point for node version, My suggestion is there is no need to limit the support for the engine unless the engine break compatibility with the lib apis. Just like all other libs graphql, etc... but it's your choice.

For the type script, maybe it would handy and easier if there is declaration type published and declared in package json as well, just like the ones you have inside - it makes typescript imports a lot easier - if possible.

Thank you again for the effort, discussion and reply 🙏

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