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

feat: add deno support, alt approach #14386

Closed
wants to merge 8 commits into from
Closed

Conversation

kt3k
Copy link
Contributor

@kt3k kt3k commented Jul 19, 2022

This is an alternative approach to #14250

This PR updates prisma generate command. When previewFeatures = ["denoDeploy"] is specified in the schema, then generate command outputs the client compatible with Deno and Deno Deploy.


Summary of changes (all changes only happen when previewFeatures = ["denoDeploy"] is specified):

  • Updates the generated client scripts to fit to deno
  • Generates runtime/edge-esm.js which is used by deno
  • Shows the error message when output is not specified in generator client
  • Shows the example which is suitable for deno (import ... from './client/edge.ts' instead of import ... from './client/edge')

@kt3k kt3k requested a review from a team July 19, 2022 16:14
@kt3k kt3k requested review from Jolg42 and jkomyno as code owners July 19, 2022 16:14
@kt3k kt3k requested review from aqrln and removed request for a team July 19, 2022 16:14
@Jolg42 Jolg42 requested a review from millsp July 19, 2022 16:16
@kt3k
Copy link
Contributor Author

kt3k commented Jul 21, 2022

I believe the CI failures are caused by the flakiness of test cases. I found similar errors in main's CI

  • The error in @prisma/client tests/functional/missing-env/tests.ts 1 2
  • The error in @prisma/generator-helper src/__tests__/generatorHandler.test.ts 3

@kt3k
Copy link
Contributor Author

kt3k commented Sep 1, 2022

@millsp Updated the PR. This branch now uses previewFeatures = ["denoDeploy"] and doesn't affect existing prisma clients. PTAL

Summary of changes (all changes only happen when previewFeatures = ["denoDeploy"] is specified):

  • Updates the generated client scripts (edge.js and runtime/index.d.ts) to fit to deno
  • Generates deno specific scripts (edge.ts (thin wrapper), deno-polyfill.js (small polyfill))
  • Generates runtime/edge-esm.js which is used by deno
  • Shows the error message when output is not specified in generator client
  • Shows the example which is suitable for deno (import ... from './client/edge.ts' instead of import ... from './client/edge')

@kt3k
Copy link
Contributor Author

kt3k commented Sep 1, 2022

BTW I currently can't build main branch (on my local machine) because of missingDependencies ["is-ci"] error in pnpm run build of packages/internals. ( I created this diff from b39cf85 )

@Jolg42
Copy link
Member

Jolg42 commented Sep 1, 2022

@kt3k we recently removed the is-ci dependency from internals.
Though it should not be used there anymore, maybe try to get the branch up-to-date with main.
Make sure to run pnpm i at the root directory.

@kt3k
Copy link
Contributor Author

kt3k commented Sep 1, 2022

I now see an error in my integration test setting (like ENOENT: no such file or directory, open '/path/to/prisma/reproductions/basic-sqlite/prisma/generated/client/deno/index.d.ts'. I'll address that tomorrow

@millsp
Copy link
Member

millsp commented Sep 1, 2022

I pinged you on slack about the deno folder, so that we can have a quick conversation about it.

@kt3k
Copy link
Contributor Author

kt3k commented Sep 2, 2022

I now see an error in my integration test setting (like ENOENT: no such file or directory, open '/path/to/prisma/reproductions/basic-sqlite/prisma/generated/client/deno/index.d.ts'. I'll address that tomorrow

Addressed this issue. The generated client can make request to data proxy in my settings. It also can pass the type checking by deno's type checker (type completions of prisma client, models, etc are available).

Now the prisma generate creates directory /deno/, and deno specific files are created in it. The main entrypoint for deno deploy client is ${output}/deno/deploy.ts for now. I'm open to change it back to the root if we prefer it. What do you think?

@janpio
Copy link
Member

janpio commented Oct 11, 2022

(Sorry, I tried to resolve the conflicts and had messed up - so had to force push the previous state again. No changes.)

Comment on lines +260 to +268
} else {
hint = `You can now start using Prisma Client in your code. Reference: ${link('https://pris.ly/d/client')}
${chalk.dim('```')}
${highlightTS(`\
import { PrismaClient } from '${importPath}/deno/edge.ts'`)}
${chalk.dim('```')}

You will need a Prisma Data Proxy connection string. See documentation: ${link('https://pris.ly/d/data-proxy')}
${breakingChangesStr}${versionsWarning}`
Copy link
Member

Choose a reason for hiding this comment

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

I don't like that deno previw is in the else. Can we rewrite this that this also reacts to isDenoPreview @millsp?

@janpio
Copy link
Member

janpio commented Oct 14, 2022

New engine to use with deno preview feature: 4.5.0-38.71723d0f37369be033e71bd7cd314749d8348f9f

@millsp
Copy link
Member

millsp commented Oct 15, 2022

Work continued here #15281

@millsp
Copy link
Member

millsp commented Oct 17, 2022

We've merged this work via #15281. Kudos to @kt3k, we're super thankful for your contribution and good work! 🥳 🎉

@millsp millsp closed this Oct 17, 2022
@kt3k
Copy link
Contributor Author

kt3k commented Oct 17, 2022

@millsp So glad to see this landed! Thank you for your reviews and guidances!

@kt3k kt3k deleted the feat-deno-support-2 branch October 17, 2022 16:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants