Skip to content
This repository has been archived by the owner on Jan 14, 2021. It is now read-only.

Facade package @prisma/photon #261

Closed
6 tasks
schickling opened this issue Oct 17, 2019 · 20 comments
Closed
6 tasks

Facade package @prisma/photon #261

schickling opened this issue Oct 17, 2019 · 20 comments
Assignees
Labels
kind/improvement An improvement to existing feature and code.
Milestone

Comments

@schickling
Copy link
Member

schickling commented Oct 17, 2019

This issue describes the idea to introduce a new NPM package published on @prisma/photon to solve current usability problems and make Photon.js more resilient in regards to build and deployment workflows. (Note: This issue also assumes the path of the generated Photon.js library is changed from @generated/photon to @prisma/photon as described in #88. The mentioned @prisma/photon package below has nothing to do with the currently existing NPM package which is an implementation detail.)

1. Current problems

There are currently a few friction points in regards to the "generated node_modules package nature" of Photon.js as follows:

1.1. Users are currently required to add "postinstall": 'prisma2 generate'" to their scripts in their package.json file to make sure Photon.js is re-generated after e.g. npm install has been running. This is an additional friction point that we should remove as it clutters a users package.json file and in the case of a user forgetting to add it will lead to breaking a users app.

1.2. Even with a postinstall in place there still seems to be an issue with packages being pruned (see #88 (comment)).

2. Solution

The solution for the problems above could be to introduce a "facade package" published on NPM as @prisma/photon which the user adds to their package.json as any other regular package. This "facade package" doesn't come with the actual Photon.js library (i.e. no runtime/types) but is basically just a "smart placeholder" to solve the problems above and provide a better user experience. The user would still need to run prisma2 generate (initially and whenever their Prisma schema changes) however Photon.js wouldn't be deleted anymore when the user runs e.g. npm install and "survives pruning".

Running prisma2 generate will overwrite the facade package and basically replace it with a functional version of Photon.js. (The specifics of this are TBD. It could also simply add e.g. a generated folder in the facade package which makes it functional. Engineering will have to figure out the details here.)

To make things even smoother the @prisma/photon package can provide its own postinstall hook (i.e. not required in the user's package.json) which could do the following when a user runs npm install:

  • Check whether Photon.js needs to be generated. If yes, proceed, otherwise do nothing.
  • Detect whether prisma2 is available in the $PATH (either globally or via NPM) and call prisma2 generate. Otherwise print a helpful message in the terminal that the user still needs to run prisma2 generate
  • The same message should also be printed when the user runs their Node app without having run prisma2 generate yet.

3. Considerations

  • We need to verify that the solution above is actually also an improvement in regards to the mental model of users but I'm feeling pretty positive about this as it generally makes the setup more transparent and resilient.
  • To make sure changes are picked up in tooling relying on live-reloading (e.g. VSC, ts-node-dev, ...) we should keep using the "delete and re-create" trick we're currently using.
  • Versions of the facade package and the Prisma Framework need to be kept up to date. Maybe we can check for this as part of the postinstall step.
  • We could automatically download the query engine binary as part of the facade package.
  • We could consider making npm install @prisma/photon the primary getting started experience. However, the user would still need the prisma Framework CLI as a dev dependency.
  • How does this affect the output config option for the photonjs generator.

4. Related issues

5. Further notes

In order to make the generation step optional, I also considered to split up Photon.js into an optional generated @types/@prisma/photon types component (same as nexus-prisma does) and a separate runtime library component published on NPM. However this doesn't work as you still need a generation step to generate the "DMMF" part of Photon.js which isn't really an improvement. I still wanted to document these thoughts here regardless. 🤷‍♂

@schickling schickling added the kind/improvement An improvement to existing feature and code. label Oct 17, 2019
@schickling schickling pinned this issue Oct 17, 2019
@schickling schickling added the process/candidate Candidate for next Milestone. label Oct 17, 2019
@peterp
Copy link

peterp commented Oct 17, 2019

Awesome, I think this would feel very idiomatic.

@jasonkuhrt
Copy link

jasonkuhrt commented Oct 17, 2019

With @prisma/photon package that encompasses the runtime client other npm libs/tools like nexus-prisma can remove what is currently a silently confusing and a leaky abstraction. Namely when pinning e.g.:

https://github.com/prisma-labs/nexus-prisma/blob/master/package.json#L23

  "dependencies": {
    "@prisma/photon": "^0.2.94",

for e.g. types:

https://github.com/prisma-labs/nexus-prisma/blob/fe6ea99616c57d9d66a67ba59b5e4c08d38b4701/src/dmmf/utils.ts#L1

import * as Photon from '@prisma/photon'
import { transform } from './transformer'
import { DMMFClass } from './DMMFClass'

export function fromPhotonDMMF(photonDMMF: Photon.DMMF.Document): DMMFClass {
  return new DMMFClass(transform(photonDMMF))
}

We're pinning against the generator but what we're actually interested in is the runtime.

Ideally libs should be able to depend on a pinned version of @prisma/photon to get the static types statically from it, yet also come generation time have the corresponding runtime (should be of same version) be generated for their app's runtime. And there should never be any surprises here.

My conclusion from the above requirement is that @prisma/photon should be both the generator and the runtime. The published package contains a runtime facade and the generator engine. This way when nexus-prisma pins to @prisma/photon@1.2.3 (or peer dep) it can be sure that its going to get 1.2.3 runtime. At the same time, it can statically import the 1.2.3 types like dmmf or maybe the photon class reference.

Maybe I'm missing something, but this seems to be more than just about DX then, but reliability.

@peterp
Copy link

peterp commented Oct 17, 2019

@jasonkuhrt My understanding is that the runtime is downloaded during the generation step, and what is generated/ downloaded is determined by the version of prisma2.

Of course that may be false, but that's the mental model that I'm currently working with.

@peterp
Copy link

peterp commented Oct 17, 2019

@jasonkuhrt Apologies. I didn't grasp the full context of what you're suggesting during my first read. I now understand that you want to pin against a specifically generated version of the runtime and the client in a 3rd party lib as a peer dependency.

@jasonkuhrt
Copy link

jasonkuhrt commented Oct 17, 2019

Related prisma/specs#152

@divyenduz
Copy link

divyenduz commented Oct 18, 2019

The first hiccup I ran into when trying this out. I made an empty package that runs prisma2 generate on postinstall, the issue is that postinstall hook as process.cwd pointed at the package path

image

Figuring out a way to override this.

Update: Take 1 at fixing this - prisma/prisma#789

divyenduz pushed a commit to prisma/prisma that referenced this issue Oct 18, 2019
When prisma2 generate is run from within node_modules, npm scripts provide that path as the process.cwd() path

However, it also provides the init cwd in this environment variable: https://docs.npmjs.com/cli/run-script

Related issue: prisma/prisma-client-js#261
@divyenduz
Copy link

divyenduz commented Oct 21, 2019

Notes from further exploration on Heroku.

./app
./app/package.json --> prisma2 dependency declared here 
./app/node_modules/photon-placeholder
├── package.json --> prisma2 referenced here without being a dependency (maybe a peerDependency)

With this setup, Heroku has a race condition between running the postinstall hook in photon-placeholder and installation of prisma2 CLI. It works sometimes but yields prisma2 not found most of the times.

As a temporary workaround, I am using npm prisma2@alpha generate as the postinstall hook in photon-placeholder. That works.

Also to note that Heroku has defaults for stripping devDependencies, if we want this setup to work, we have to either disable those (NODE_ENV not to be set to production etc) or declare prisma2 in dependencies once we sort this race condition issue out.

CleanShot 2019-10-21 at 15 00 50@2x

Sample Photon placeholder: https://github.com/divyendu-test/photon-placeholder
Sample Heroku project that uses this facade: https://github.com/divyendu-test/p2-facade

@divyenduz divyenduz added this to the Preview 15 milestone Oct 21, 2019
@jasonkuhrt
Copy link

jasonkuhrt commented Oct 21, 2019

Also to note that Heroku has defaults for stripping devDependencies, if we want this setup to work, we have to either disable those (NODE_ENV not to be set to production etc) or declare prisma2 in dependencies once we sort this race condition issue out.

But first Heroku installs everything, runs npm run build (if present). Once photon has been built, prisma 2 will not be needed anymore correct? I don't see the issue? Probably missing something 🤔

@divyenduz
Copy link

divyenduz commented Oct 22, 2019

@jasonkuhrt Great catch, just ran into this: https://help.heroku.com/P5IMU3MP/heroku-node-js-build-script-change-faq

This means using "build" in place of "postinstall" might work for Heroku (still not a generic solution) 👍
Experimenting further and will share my findings.

Update: This only works for app level "build", doesn't change much for a sub-package (photon-placeholder) that wants to run prisma2 generate

@darknoon
Copy link

This looks like an interesting direction, all I would as is that you check that it works with yarn workspaces, where packages might be hoisted to the root.

Theoretically you could have 2 divergent @prisma/photon packages in separate workspaces that alias to the same folder, though I don't depend on this working at the moment.

@nikolasburk
Copy link
Member

nikolasburk commented Nov 21, 2019

Update: The docs update is in progress and will be finished by tomorrow. Breaking change will be prominently highlighted in release notes.

@kripod
Copy link
Contributor

kripod commented Nov 22, 2019

@nikolasburk Sounds great! I may be missing something obvious, but where can I find the changelog?

It turns out that the release notes can be found under the prisma2 repo.

@timsuchanek
Copy link
Contributor

Implemented in preview release 17.
You can try it out with npm i prisma2 @prisma/photon.

@2color
Copy link
Contributor

2color commented Nov 26, 2019

Naive question: what is the reason for generating the photon library in node_modules?

Is that to ensure it's not committed into SCM given that it's normally generated from the Prisma file (which is generally managed from within the SCM)?

@jasonkuhrt
Copy link

@2color the generation result is based on the contents of the users' schema.prisma file. Inherently dynamic, not something that could ever live in scm.

@nikolasburk
Copy link
Member

@2color we also have a short section about this in the docs: https://github.com/prisma/prisma2/blob/master/docs/photon/codegen-and-node-setup.md#why-is-photonjs-generated-into-node_modulesprismaphoton-by-default

TLDR:

  • the mental for using Photon.js is "using another npm module"
  • convenient imports
  • keeping the query engine out of version control by default

image

@jasonkuhrt
Copy link

@2color i think I misunderstood your question. You were talking about user’s project scm, I missed that. Anyways @nikolasburk gave a great answer!

@Johannes-B-stock
Copy link

I'm having an issue with deploying a prisma client to heroku. The client is generated in postinstall (default to node_modules) but after herokus pruning and starting the app I get following error: "Error: @prisma/client did not initialize yet. Please run "prisma generate" and try to import it again."
I'm quite confused, since prisma client is in dependencies and not devDependencies, does anyone have a hint or solution for me?

@peterp
Copy link

peterp commented Apr 29, 2020

@Johannes-B-stock you can modify the location of the generated client: https://www.prisma.io/docs/reference/tools-and-interfaces/prisma-schema/generators

Maybe that'll work, and then you can be sure that it's the pruning step.

@divyenduz
Copy link

@Johannes-B-stock We also have this example on Heroku from our E2E tests https://github.com/prisma/prisma2-e2e-tests/tree/master/platforms/heroku

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
kind/improvement An improvement to existing feature and code.
Projects
None yet
Development

No branches or pull requests