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

Next example doesn't build #71

Open
6 tasks done
hyperknot opened this issue Jan 17, 2023 · 9 comments
Open
6 tasks done

Next example doesn't build #71

hyperknot opened this issue Jan 17, 2023 · 9 comments
Labels
bug Something isn't working

Comments

@hyperknot
Copy link

hyperknot commented Jan 17, 2023

Preflight checklist

Describe the bug

build throws the following errors:

  • @ory/client missing
  • axios missing
  • and a TS error:
./pages/login.tsx:104:21 at setFlow(err.response?.data)
Type error: Argument of type 'unknown' is not assignable to parameter of type 'SetStateAction<LoginFlow | null>'.

There might be more TS errors, it stops at the first one.

Reproducing the bug

run build

Relevant log output

No response

Relevant configuration

No response

Version

master

On which operating system are you observing this issue?

None

In which environment are you deploying?

None

Additional Context

No response

@hyperknot hyperknot added the bug Something isn't working label Jan 17, 2023
@Benehiko
Copy link
Contributor

Benehiko commented Jan 17, 2023

Hi @hyperknot

Please check if you have done the following:

cd elements
npm run initialize
cd examples/nextjs-spa
npm run dev

note: you do not need to run npm i inside of the example folder since the packages are hoisted up to the root directory of elements - managed by Lerna.

@hyperknot
Copy link
Author

hyperknot commented Jan 17, 2023

I'm using pnpm, I haven't invested into how to make it work with Lerna. But aren't the examples supposed to be "self-sufficient" that is, they can be moved outside the repo and work on their own? For example Next.js has over 300 examples, and all of them are working independently.
https://github.com/vercel/next.js/tree/canary/examples

@Benehiko
Copy link
Contributor

You can use normal npm.

The examples are linked to the latest elements build, which exists through the Lerna build process. This makes it easy for us to test and maintain within the same repository. You can of course move the nextjs example outside of this repositories scope and just install the missing packages manually. There shouldn't be that many since the examples are also quite lean.

To understand how this works inside of the elements repository, see here https://github.com/ory/elements/blob/main/examples/nextjs-spa/package.json#L17
Take note of the * for the package version. Lerna handles this for us by using a local link instead of a version on the npm registry.

We also documented this inside the readme https://github.com/ory/elements#contributing and https://github.com/ory/elements#understanding-ory-elements

@hyperknot
Copy link
Author

hyperknot commented Jan 17, 2023

I understand. It's an easy fix anyway, just 2-3 packages installed is no big deal.

Do you know why is the TS error? Is it related to something with lerna or build also throws this on your side?

@Benehiko
Copy link
Contributor

Benehiko commented Jan 17, 2023

I don't get any ts errors 😕

I would recommend you delete the node_modules folder and reinstall the packages. Below commands should do the trick:

cd elements
npm run clean
npm run initialize
cd examples/nextjs-spa
npm run dev

you can replace npm run dev with npm run build for a production build of nextjs.

@Benehiko
Copy link
Contributor

unless you copied out the nextjs example and don't have eslint setup correctly 🤔

@hyperknot
Copy link
Author

hyperknot commented Jan 17, 2023

It works with the lerna steps, no error. But are these using non-standard ESLint configurations? I mean I guess every user who encounters this repository will want to run / copy these into their personal projects. For example the Next.js one should compile with the standard ESLint coming from create-next-app, otherwise it'd be a lot of effort just to try to make it run.

@hyperknot
Copy link
Author

hyperknot commented Jan 18, 2023

I went through and integrated Ory Elements into my Next.js project. The example is basically copy-pasted, except for these errors, which all have to be @ts-ignore-ed, one-by-one.

I believe the Next.js setup is more strict then what you are using for developments. For a nicer DX I'd recommend using the same settings as Next.js so that users don't have to manually @ts-ignore them.

As an alternative, having a higher level wrapper for <OryLogin oryClient={} config={}> would be a very nice DX. Only a single, one-line component for each scenario.

@Benehiko
Copy link
Contributor

Thank you for the feedback! I will take a look in the coming days on how we can improve this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants