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

refactor(framework): Make discovery completely asynchronous #6879

Merged
merged 6 commits into from
Nov 8, 2024

Conversation

rifont
Copy link
Collaborator

@rifont rifont commented Nov 7, 2024

What changed? Why was the change needed?

  • Make discovery completely asynchronous
    • Asynchronous discovery is necessary to allow for use of dynamic awaitable imports (await import(...)) of peer dependencies to provide an optimal DX, where no manual dependency injection is required.
  • Only log out discovered workflows that are served (move the prettyPrintDiscovery to the client. This also lays the groundwork for making the pretty discovery use a custom logger

Screenshots

next dev --turbopack compatibility is a step closer now, erroring out with module not found rather than the previous require polyfill error.Some further investigation is to identify the root cause for the new problem. Judging by the docs at https://nextjs.org/docs/messages/module-not-found, it's probably:
The module you're trying to import uses Node.js specific modules, for example dns, outside of getStaticProps / getStaticPaths / getServerSideProps. Here is the current usage of node built-in modules (node:crypto is used at runtime for HMAC header validation, node:util/types is used for an error type guard).

> next dev --turbopack --port 4000

   ▲ Next.js 15.0.2 (Turbopack)
   - Local:        http://localhost:4000
   - Environments: .env.local

 ✓ Starting...
 ✓ Ready in 748ms
 ○ Compiling /api/novu ...
 ✓ Compiled /api/novu in 555ms
 ⨯ ./src/app/api/novu/route.ts:2:1
Module not found: Can't resolve '@novu/framework'
  1 | import { serve } from '@novu/framework/next';
> 2 | import { workflow } from '@novu/framework';
    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  3 |
  4 | const myWorkflow = workflow('welcome-onboarding-2', async ({ step }) => {
  5 |   await step.inApp('send-in-app', async () => ({ body: 'Welcome to our platform!', }));



https://nextjs.org/docs/messages/module-not-found
Expand for optional sections

Related enterprise PR

Special notes for your reviewer

Copy link

netlify bot commented Nov 7, 2024

Deploy Preview for novu-stg-vite-dashboard-poc failed. Why did it fail? →

Name Link
🔨 Latest commit 51200c6
🔍 Latest deploy log https://app.netlify.com/sites/novu-stg-vite-dashboard-poc/deploys/672d3efc76e9e8000878fbe3

try {
// eslint-disable-next-line global-require
const { zodToJsonSchema } = require('zod-to-json-schema') as typeof import('zod-to-json-schema');
const { zodToJsonSchema } = await import('zod-to-json-schema');
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Begone, dynamic require!

@rifont rifont requested a review from djabarovgeorge November 7, 2024 10:58
Copy link

pkg-pr-new bot commented Nov 7, 2024

Open in Stackblitz

pnpm add https://pkg.pr.new/novuhq/novu/@novu/framework@6879

commit: 51200c6

@rifont rifont merged commit 6a45807 into next Nov 8, 2024
36 of 40 checks passed
@rifont rifont deleted the async-framework-workflows branch November 8, 2024 00:08
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.

2 participants