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

Update tooling #169

Merged
merged 5 commits into from
Apr 18, 2024
Merged

Update tooling #169

merged 5 commits into from
Apr 18, 2024

Conversation

djhi
Copy link
Contributor

@djhi djhi commented Apr 15, 2024

  • Update more dependencies

  • Switch to vite

  • Fix CLI

  • Check the CLI mode is working

  • Check the import through bundlers is working

  • Check the import inside Node is working

@djhi djhi added the RFR Ready For Review label Apr 15, 2024
src/introspection/getSchemaFromData.spec.js Show resolved Hide resolved

test: ## Launch unit tests
@NODE_ENV=test ./node_modules/.bin/jest
@NODE_ENV=test NODE_OPTIONS="$$NODE_OPTIONS --experimental-vm-modules" ./node_modules/.bin/jest
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this I don't understand. Would you ind explaining what it does?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make jest accept our ESM code: https://jestjs.io/docs/ecmascript-modules

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does that mean that, in order to use the CLI, our users will have to use the same flag? This would be a BC.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nope, as said in the PR desc, I tested the CLI. I did it without any flag on latest Node LTS

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The minimum required node version is 16.

bin/json-graphql-server.cjs Outdated Show resolved Hide resolved
vite.config.js Show resolved Hide resolved
bin/json-graphql-server.cjs Outdated Show resolved Hide resolved
@djhi djhi added WIP RFR Ready For Review and removed RFR Ready For Review WIP labels Apr 16, 2024
src/introspection/getSchemaFromData.spec.js Show resolved Hide resolved
vite.config.umd.js Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
@@ -448,15 +448,15 @@ Then use the `jsonGraphqlExpress` express middleware:

```js
import express from 'express';
import jsonGraphqlExpress from 'json-graphql-server';
import jsonGraphqlExpress from 'json-graphql-server/node';

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
import jsonGraphqlExpress from 'json-graphql-server/node';
const { default: jsonGraphqlExpress } = require("json-graphql-server/node");

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I reverted the change, the doc was right initially

@fzaninotto fzaninotto merged commit 7f32987 into master Apr 18, 2024
1 check passed
@fzaninotto fzaninotto modified the milestones: 2.4.1, 3.0 Apr 18, 2024
@fzaninotto fzaninotto mentioned this pull request Apr 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
RFR Ready For Review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants