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

Updates for DDN Alpha and TypeScript Deno connector compatibility #1

Merged
merged 3 commits into from
Nov 30, 2023

Conversation

daniel-chambers
Copy link
Contributor

@daniel-chambers daniel-chambers commented Nov 29, 2023

This PR updates this repo to be compatible with the DDN Alpha and also with the latest TypeScript Deno connector compatibility (namely the improved watch support, introduced in hasura/ndc-typescript-deno#79).

  • Added a workaround that ensures the errors property is not nullable/undefined on the return object, because the TypeScript Deno connector doesn't support nullable/undefined types properly yet
  • Dockerfile updated to match latest TypeScript Deno version
  • config.json file deleted and an empty config just inlined into the CLI commands in the readme
  • The metadata.hml has been replaced with a full DDN project structure (hasura.yaml, build-profile.yaml, etc)
  • Readme has been updated with latest Hasura CLI syntax
  • VSCode extensions.json has been added to recommend the required VSCode extensions
  • VSCode settings.json added to ensure Deno extension is activated in this repo

@daniel-chambers daniel-chambers self-assigned this Nov 29, 2023
Copy link
Contributor

@sordina sordina left a comment

Choose a reason for hiding this comment

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

Looks good to me. The subgraph stuff certainly adds a lot of files, but given that this is primarily an example repo I think that's fine. If only there was some way to make sure it stayed in sync with any future format changes...

{
"deno.enable": true,
"deno.lint": true,
"deno.unstable": false
Copy link
Contributor

Choose a reason for hiding this comment

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

We're using unstable features in the connector to support the NPM modules so should we try to match this with the dev tools?

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'm not sure that refers to that, I think this enables access to Deno's unstable APIs. I've used an NPM package (for testing) in this project with that set to false and it works just fine.

COPY ./functions /functions/src

# Pre-cache inference results and dependencies
RUN PRECACHE_ONLY=true /app/entrypoint.sh
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

@daniel-chambers
Copy link
Contributor Author

daniel-chambers commented Nov 29, 2023

I tested this against DDN Production and it returns a response that is making the engine choke.

image

More investigation required before merging.

@daniel-chambers daniel-chambers force-pushed the daniel/update-for-latest-connector branch from 75d3ba7 to cc37e1d Compare November 30, 2023 02:21
@daniel-chambers daniel-chambers merged commit 739e068 into main Nov 30, 2023
@daniel-chambers daniel-chambers deleted the daniel/update-for-latest-connector branch November 30, 2023 02:23
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

Successfully merging this pull request may close these issues.

2 participants