Blitz: Importing from a file in pages/ or api/ fails because it can't resolve it

[Skn0tt]

What is the problem?

From a mutation, I want to import an API route (that's required to make quirrel.dev's API work).
Since Blitz.js moves /api into /pages, though, this doesn't work.

There are two solutions that come to my mind:

• rewrite all imports from app/api/... to pages/api/...
• create a file in app/api/... that contains module.exports = require("pages/api/...")

Steps to Reproduce

1. Check out https://github.com/quirrel-dev/quirrel-blitz-repro
2. blitz start

Versions

debug: blitzPkgPath: /home/codespace/workspace/quirrel-blitz-repro/node_modules/blitz/dist/index.js
debug: cliPkgPath: /home/codespace/workspace/quirrel-blitz-repro/node_modules/@blitzjs/cli/lib/src/index.js 

Linux 5.4 | linux-x64 | Node: v12.16.3

blitz: 0.21.1 (global)
blitz: 0.21.1 (local)

  Package manager: yarn 
  System:
    OS: Linux 5.4 Debian GNU/Linux 9 (stretch) 9 (stretch)
    CPU: (2) x64 Intel(R) Xeon(R) Platinum 8168 CPU @ 2.70GHz
    Memory: 680.01 MB / 3.82 GB
    Shell: 4.4.12 - /bin/bash
  Binaries:
    Node: 12.16.3 - /opt/nodejs/lts/bin/node
    Yarn: 1.17.3 - /opt/yarn/stable/bin/yarn
    npm: 6.14.4 - /opt/nodejs/lts/bin/npm
    Watchman: Not Found
  npmPackages:
    @prisma/cli: 2.6.2 => 2.6.2 
    @prisma/client: 2.6.2 => 2.6.2 
    blitz: 0.21.1 => 0.21.1 
    react: 0.0.0-experimental-7f28234f8 => 0.0.0-experimental-7f28234f8 
    react-dom: 0.0.0-experimental-7f28234f8 => 0.0.0-experimental-7f28234f8 
    typescript: 3.9.7 => 3.9.7 

Other

error - ./app/_resolvers/mutations/invokeQueue.ts:1:0
Module not found: Can't resolve 'app/api/emailQueue'
> 1 | import emailQueue from "app/api/emailQueue"
  2 | 
  3 | export default async function invokeQueue() {
  4 |   await emailQueue.enqueue()
event - build page: /404
wait  - compiling...

[aem]

rewrite all imports from app/api/... to pages/api/...

That seems like the ideal solution if we want to support this, but I think there's also a valid case for suggesting that you shouldn't be importing from /api, you should be pulling the logic there into a utility function that can be imported both in the /api directory and in your app code. Any thoughts there?

[Skn0tt]

While I‘m generally with you, I think I have a valid case here. I need this to make Quirrel‘s API work, which uses API routes in the following way:

// /api/emailQueue.js
import { Queue } from "@quirrel/next"

export default Queue(
  "emailQueue",
  async payload => {
    await email.send( ... )
  }
)

That can the be imported in e.g. a mutation:

// mutations/signup.js
import emailQueue from "../api/emailQueue"

export default async (req, res) => {
  await createUser(...);

  await emailQueue.enqueue({
    recipient: req.body.email,
    subject: "How was your first day with Blitz?",
    ...
  }, {
    delay: "1d"
  })
}

[ryardley]

I would argue that that is not really business logic and should probably be middleware

[Skn0tt]

Apart from my specific use-case, I‘m pretty sure people would be surprised about the behavior. „This works in Next.js, why doesn‘t it in Blitz?“

The way this breaks is non-obvious to most users.
If the file pipeline moves files around, it should also do smth about the imports, Right? 😅

[ryardley]

It’s breaking a desirable architectural boundary. It should actually probably just error nicely. A mutation or query should only make calls to a database or subsequent downstream service. If you need to call api routes you can always call them using http. Alternatively pulling from a shared file makes sense the way @aem was suggesting.

[aem]

To be honest, I'm not sure Next.js should even support this. There's probably some pretty bad side-effects that could occur (e.g. security problems) by importing an entire API endpoint into application code, and vice versa. Something like this probably makes more sense:

// /app/queues/emailQueue.ts
import { Queue } from "@quirrel/next"

export default Queue(
  "emailQueue",
  async payload => {
    await email.send( ... )
  }
)

// /app/api/emailQueue.ts
export * from "app/queues/email-queue.ts"

// /app/mutations/signup.ts
import emailQueue from "app/queues/email-queue"

export default async (req, res) => {
  await createUser(...);

  await emailQueue.enqueue({
    recipient: req.body.email,
    subject: "How was your first day with Blitz?",
    ...
  }, {
    delay: "1d"
  })
}

[Skn0tt]

@timneutkens is this intentionally supported by Next.js?

[flybayer]

(Btw, this also affects importing things from inside any pages folder.)

I'm 100% with @Skn0tt on this one.

@ryardley and @aem your opinions on architecture are valid and good, but I firmly believe it's not Blitz's place to enforce architecture at this level. It's one thing to suggest and recommend a certain architecture, but it's a whole another level to enforce it.

I feel very strongly, that as a guiding principle, our compile step should not break anything you can do without our compile step. This issue at hand is directly because of how the compile step moves files around. Not being able to do something because the compiler breaks the file layout is quite frustrating. A similar issue is https://github.com/blitz-js/blitz/issues/734

The other really good point on this is that this works fine in Next.js. We've emphasized that we are remaining 100% compatible with Next.js. If we don't fix this import issue, then we can't say that. If everyone want's to stop being compatible with Next.js, that's fine, but this needs to be brought up as dedicated discussion.

In the past I had to import the handler from one API route and re-use it for another API route before Next.js added the [[...slug]] feature. Don't need it for that specific use case now, but undoubtably someone is going to need to re-use a handler for something.

Another common use case that has nothing to do with app architecture: it's quite common to define a Typescript type in a Page file and then import that type somewhere else. Another one is defining a context in a page, and then importing and using that context elsewhere. Currently this is broken.

On a philosophical level, I definitely align with the Rails doctrine of "provide sharp knives". We should give people the sharpest knives possible and trust them to use the knives in the way that get's the job done best for them. The opposite of this is making Fisher Price toys that are so safe they can't be used for any real work. And again here, what we have as defaults for people is one thing that should be relatively safe. But we shouldn't enforce what we think is safest or best because without fail doing so will prevent someone from doing something they have a valid use case for.

[carlosbaraza]

I encountered a similar problem when importing a variable declared in a page file from a component. This could be fixed in multiple ways, but it is just strange and confusing.

// file path: app/projects/component/ProjectsList.tsx

// This throws a module not found error for Blitz, but it is accepted by the IDE as a valid path.
import { ITEMS_PER_PAGE } from "../pages/projects/index"

// This works for Blitz, but it is not found by the IDE.
import { ITEMS_PER_PAGE } from "pages/projects/index"

[Skn0tt]

I'd like to take a shot at fixing this. @aem @ryardley could you give me some clues as to where I have to look and how you'd implement this?

[flybayer]

@Skn0tt probably create a new stage: https://github.com/blitz-js/blitz/tree/canary/packages/server/src/stages