Migrate to nextjs app router #695

essential-breads · 2024-09-19T17:06:52Z

Updates the project starter to use the latest version of Nextjs with app router and no src directory. Removes the js project option. Closes
#583
#400

45930

The point of using app router is to enable server-side rendering right? Do users need to be aware of which routes and components use SSR and avoid o1js in them?

45930 · 2024-09-19T18:18:59Z

src/lib/project.js

    '--import-alias "@/*"',
-    '--no-app',
+    '--app',


Should we add this change to the changelog?

45930 · 2024-09-19T18:34:51Z

src/lib/ui/next/customNextPage.js


 export default function Home() {
  useEffect(() => {
    (async () => {
      const { Mina, PublicKey } = await import('o1js');
-      const { Add } = await import('../../../contracts/build/src/');
+      const { Add } = await import('../../contracts/build/src/');

      // Update this to use the address (public key) for your zkApp account.
      // To try it out, you can try this address for an example "Add" smart contract that we've deployed to


Did you test if uncommenting this code works with your changes?

The code doesn't error, but it's also not really doing anything. Looking at this more closely it seems like that zkapp address might be out of date. Maybe we should either remove the example code or update it to really interact with the network. What do you think?

const { Mina, PublicKey,fetchAccount } = await import('o1js'); const { Add } = await import('../../contracts/build/src/'); // connect Mina to testnet const Network = Mina.Network( 'https://api.minascan.io/node/devnet/v1/graphql' ); Mina.setActiveInstance(Network); // address of deployed Add contract on testnet https://minascan.io/devnet/account/B62qnTDEeYtBHBePA4yhCt4TCgDtA4L2CGvK7PirbJyX4pKH8bmtWe5 const zkAppAddress = `B62qnTDEeYtBHBePA4yhCt4TCgDtA4L2CGvK7PirbJyX4pKH8bmtWe5`; await fetchAccount({publicKey: zkAppAddress}); const zkApp = new Add(PublicKey.fromBase58(zkAppAddress)); console.log("Current num: ", zkApp.num.get().toString());

Yeah, sounds like a different PR though 👍

We can reconsider what the right format is for demoing o1js in a boilerplate app.

The contract is currently deployed on devnet. https://minascan.io/devnet/account/B62qnTDEeYtBHBePA4yhCt4TCgDtA4L2CGvK7PirbJyX4pKH8bmtWe5?type=zk-acc

This example code was originally added to demonstrate how to import a contract into a UI. The goal was to eventually add further interactions in this example boilerplate.

B62qnTDEeYtBHBePA4yhCt4TCgDtA4L2CGvK7PirbJyX4pKH8bmtWe5 is the one that I deployed during testing, I will replace the old address with this one in all of the cli files!

essential-breads · 2024-09-19T19:52:26Z

The point of using app router is to enable server-side rendering right? Do users need to be aware of which routes and components use SSR and avoid o1js in them?

One of the reasons Next moved to app router was to better support React's server components. Info explaining usage with server components may be well suited to a Next.js specific section in the docs where we also call out things like loading the SDK in useEffect. The user will not be able to call useEffect in a server component but could attempt to load the SDK as a top level import and run into issues if we don't guide them on best practices there.

ymekuria

Nice work! I cloned your repo and was able to successfully generate a project using the app router. I'm approving in advance, but could you change the copy on the UI landing page to match the updated app router file path?

zkapp-cli/src/lib/ui/next/customNextPage.js

Lines 60 to 61 in 512e7ed

    
                       Get started by editing 
        
                       <code className={styles.code}> src/pages/index.js</code> or <code className={styles.code}> src/pages/index.tsx</code>

ymekuria · 2024-09-19T21:15:54Z

src/lib/project.js

    '--import-alias "@/*"',
-    '--no-app',
+    '--app',


ymekuria · 2024-09-19T21:28:39Z

src/lib/ui/next/customNextPage.js


 export default function Home() {
  useEffect(() => {
    (async () => {
      const { Mina, PublicKey } = await import('o1js');
-      const { Add } = await import('../../../contracts/build/src/');
+      const { Add } = await import('../../contracts/build/src/');

      // Update this to use the address (public key) for your zkApp account.
      // To try it out, you can try this address for an example "Add" smart contract that we've deployed to


The contract is currently deployed on devnet. https://minascan.io/devnet/account/B62qnTDEeYtBHBePA4yhCt4TCgDtA4L2CGvK7PirbJyX4pKH8bmtWe5?type=zk-acc

This example code was originally added to demonstrate how to import a contract into a UI. The goal was to eventually add further interactions in this example boilerplate.

codecov · 2024-09-19T22:11:43Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.00%. Comparing base (86e13f7) to head (077e45b).

Additional details and impacted files

@@            Coverage Diff            @@
##              main      #695   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           11        11           
  Lines         1491      1475   -16     
  Branches       312       300   -12     
=========================================
- Hits          1491      1475   -16

Flag	Coverage Δ
unittests	`100.00% <100.00%> (ø)`

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

ymekuria · 2024-09-19T22:16:02Z

The point of using app router is to enable server-side rendering right? Do users need to be aware of which routes and components use SSR and avoid o1js in them?

Vercel is also pushing the app router as the new standard for NextJS. A user can chose only to use client side components with the app router if they want.

ymekuria · 2024-09-24T06:14:55Z

src/lib/project.js

  fs.writeFileSync(path.join('ui', 'next.config.mjs'), newNextConfig);

-  const indexFileName = useTypescript ? 'index.tsx' : 'index.js';
+  const pageFileName = 'page.tsx';
+


It looks like there are some legacy create-next-app boilerplate styles and fonts leftover in the app folder when a project is generated. I think it is a good idea to remove these so they do not conflict with the other assets. You could empty the app directory with something like fs.emptyDirSync or similar before adding the customNextPage and any other files to the directory.

Good catch, cleared out the whole directory

ymekuria · 2024-09-24T06:17:24Z

src/lib/project.js

  );

  // Removes create-next-app assets
  fs.emptyDirSync(path.join('ui', 'public'));
+  if (fs.existsSync(path.join('ui', 'app', 'favicon.ico'))) {


Similar comment to above.
It looks like there are some legacy create-next-app boilerplate styles and fonts leftover in the app folder when a project is generated. You could empty the entire app directory instead of just removing the favicon here with something like fs.emptyDirSync or similar before adding the customNextPage and any other files to the directory.

ymekuria · 2024-09-24T06:32:14Z

src/lib/project.js

      'utf8'
    );

-    appFile = appFile.replace(
+    pageFile = pageFile.replace(
      'export default function',
      `import './reactCOIServiceWorker';

 export default function`


@essential-breads were you able to test if the deploy to GitHub Pages flow works correctly with the appRouter changes? It might be out of scope for this PR, but I will add an issue so we don't forget to verify that everything works.

Yep GH pages is fully tested!

Katie Noland added 4 commits September 18, 2024 11:14

updates cli to use app router and skip src directory in next projects

ecf4ccb

updates the gh pages build to work with app dir

6efe2ea

removes js option from next apps

2dde882

removes testing script

5328d7d

essential-breads marked this pull request as ready for review September 19, 2024 17:29

45930 reviewed Sep 19, 2024

View reviewed changes

update change log, update unit test with latest nextjs command

512e7ed

ymekuria approved these changes Sep 19, 2024

View reviewed changes

Katie Noland added 2 commits September 19, 2024 16:27

updates path to page file

801cda2

update devnet add contract address

9de1046

ymekuria reviewed Sep 24, 2024

View reviewed changes

ymekuria mentioned this pull request Sep 24, 2024

Verify Github Pages Deploy flow works correctly #696

Open

clears default create next app assets

077e45b

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Migrate to nextjs app router #695

Migrate to nextjs app router #695

essential-breads commented Sep 19, 2024

45930 left a comment

45930 Sep 19, 2024

ymekuria Sep 19, 2024

45930 Sep 19, 2024

essential-breads Sep 19, 2024 •

edited

Loading

45930 Sep 19, 2024

ymekuria Sep 19, 2024

essential-breads Sep 19, 2024

essential-breads commented Sep 19, 2024

ymekuria left a comment •

edited

Loading

ymekuria Sep 19, 2024

ymekuria Sep 19, 2024

codecov bot commented Sep 19, 2024 •

edited

Loading

ymekuria commented Sep 19, 2024

ymekuria Sep 24, 2024

essential-breads Sep 24, 2024

ymekuria Sep 24, 2024

ymekuria Sep 24, 2024

essential-breads Sep 24, 2024

	Get started by editing
	<code className={styles.code}> src/pages/index.js</code> or <code className={styles.code}> src/pages/index.tsx</code>

Migrate to nextjs app router #695

Are you sure you want to change the base?

Migrate to nextjs app router #695

Conversation

essential-breads commented Sep 19, 2024

45930 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

essential-breads Sep 19, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

essential-breads commented Sep 19, 2024

ymekuria left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Sep 19, 2024 • edited Loading

Codecov Report

ymekuria commented Sep 19, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

essential-breads Sep 19, 2024 •

edited

Loading

ymekuria left a comment •

edited

Loading

codecov bot commented Sep 19, 2024 •

edited

Loading