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

docs: Fix import path typo in Nextjs Quickstart documentation #237

Conversation

Depender-Sethi
Copy link
Contributor

What does this PR do?

This PR addresses a minor documentation issue by correcting an import path typo in the documentation. The previous import statement incorrectly referred to "appwrite" without specifying the file path. This change ensures that the import statement points to the correct file location.

Test Plan

(Write your test plan here. If you changed any code, please provide us with clear instructions on how you verified your changes work.)

Related PRs and Issues

(If this PR is related to any other PR or resolves any issue or related to any issue link all related PR and issues here.)

Have you read the Contributing Guidelines on issues?

(Write your answer here.)

@vercel
Copy link

vercel bot commented Oct 12, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
website ✅ Ready (Inspect) Visit Preview 💬 Add feedback Oct 18, 2023 7:11pm

Copy link
Contributor

@gewenyu99 gewenyu99 left a comment

Choose a reason for hiding this comment

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

Nice catch, can you also fix these two parts:

Fix the path in

Create a new file `app/appwrite.js` and add the following code to it, replace `<YOUR_PROJECT_ID>` with your project ID.

Should be src/app/appwrite.js

Also fixapp/page.jsx, which has the same problem as above, but I think next.js uses js not jsx

@gewenyu99
Copy link
Contributor

Thanks for the help ❤️

@Depender-Sethi
Copy link
Contributor Author

Depender-Sethi commented Oct 14, 2023

Thank you for your feedback.
I've reviewed your request to fix the file paths, and I'm happy to make these changes. However, I'd like to clarify the file extension issue. Based on the latest Next.js documentation, '.js, .jsx, or .tsx' file extensions can be used for Pages. It's worth noting that typically, files containing JSX have an extension of .jsx or .tsx, especially when TypeScript is being used.

So, should I keep the extension as '.jsx' or update it to '.js' as you suggested?

PS: It's important to note that the usage of the 'src' directory is subjective. According to the Next.js documentation, the 'src' directory is optional, so even 'app/appwrite.js' and 'app/page.jsx' are valid file paths based on this flexibility.

@Depender-Sethi
Copy link
Contributor Author

@gewenyu99 I hope you're doing well. I wanted to follow up on my previous comment regarding the file extension change and the optional 'src' directory usage in Next.js. Let me know which direction to go in, and I'll push the changes as soon as possible. Please feel free to share your thoughts when you have the time. Thank you

{% section #step-5 step=5 title="Create a login page" %}
Create or update `app/page.jsx` file and add the following code to it.

```js
"use client";
import { useState } from "react";
import { account, ID } from "appwrite";
import { account, ID } from "./appwrite";
Copy link
Contributor

Choose a reason for hiding this comment

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

Please update only this line and the jsx extension, and revert all the newline and add/removed space changes in this PR to keep things concise.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've made the one-line change and kept the original spacing as per your request. It's important to note that the file is already in the .js format, so adding the .jsx extension is not required. Additionally, according to the Next.js documentation, explicit extensions like .js, .ts, .jsx, and .tsx are not necessary for importing files.

If you have any further questions or need additional assistance, please feel free to let me know.

Copy link
Contributor

Choose a reason for hiding this comment

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

Hi, this line:

Create or update `app/page.jsx` file and add the following code to it.

is using the wrong file name compared to what's generated in Next.js. Please also fix this quickly, thanks!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I misunderstood because I usually create Next.js apps with TypeScript, where the default extension is .tsx. I mistakenly assumed that in plain JavaScript, it would be .jsx. My apologies for the confusion. I've made the necessary change to page.js as you requested. Please review the updated code. If there are any further adjustments needed, please let me know.

Copy link
Contributor

Choose a reason for hiding this comment

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

No problem

@gewenyu99
Copy link
Contributor

gewenyu99 commented Oct 18, 2023

Amazing, merging after tests pass

@gewenyu99 gewenyu99 merged commit 823d64d into appwrite:main Oct 25, 2023
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants