Next 13 fixes #5175

jamesdaniels · 2022-10-26T01:44:49Z

Couple of fixes for Next.js v13

Bump deploy test to Next 13, add an app directory and some additional components
Set __NEXT_REACT_ROOT when compiling an app targeting React 18
Don't blindly copy over all .html files, we need to skip those using ISR
Skip ISR files while building the hosting directory
Generated html/json can be found in the app directory, not just pages, now
Removed some unused deps
Use NPM on Next.js hosting init

TODO

add comment clarifying we shouldn't go overboard optimizing for app directory yet, as it's unstable
change log
cleanup types
move to semver test for react 18

codecov-commenter · 2022-10-26T01:57:16Z

Codecov Report

Base: 56.22% // Head: 56.15% // Decreases project coverage by -0.06% ⚠️

Coverage data is based on head (c266bd6) compared to base (87e8f0c).
Patch coverage: 7.69% of modified lines in pull request are covered.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #5175      +/-   ##
==========================================
- Coverage   56.22%   56.15%   -0.07%     
==========================================
  Files         308      308              
  Lines       20752    20776      +24     
  Branches     4206     4214       +8     
==========================================
- Hits        11667    11666       -1     
- Misses       8071     8096      +25     
  Partials     1014     1014

Impacted Files	Coverage Δ
src/frameworks/next/index.ts	`13.41% <7.69%> (-1.59%)`	⬇️
src/emulator/auth/state.ts	`84.87% <0.00%> (-0.57%)`	⬇️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

jamesdaniels · 2022-10-28T04:32:57Z

@TheIronDev added into the nextjs deploy test. FYI I'm porting the matrix/canary tests from FirebaseExtended to this repo, WIP here #5187 aiming to have that in good shape early next week. This would catch problems like this.

scripts/webframeworks-deploy-tests/hosting/app/bar/page.tsx

inlined · 2022-10-28T17:40:39Z

src/frameworks/next/index.ts

-    // TODO drop from hosting if revalidate
-    for (const route in prerenderManifest.routes) {
-      if (prerenderManifest.routes[route]) {
+    for (const path in prerenderManifest.routes) {


Should this not be of?

it's not an iterable, it's an object: { "/foo": { ... }, "/bar": { ... }}

so in is correct, though with the linting rule (to ensure we don't iterate over private) it's a little verbose... perhaps const path of Object.keys(prerenderManifest) or const [path, route] of Object.entries(prerenderManifest.routes) would be stylistically preferable?

src/frameworks/next/index.ts

inlined · 2022-10-28T17:43:46Z

src/frameworks/next/index.ts

        // / => index.json => index.html => index.html
        // /foo => foo.json => foo.html
-        const parts = route
+        const parts = path
          .split("/")
          .slice(1)
          .filter((it) => !!it);


The slice and filter bits seem redundant. It seems like you're using splice(1) because leading slashes will lead to an empty element from split (are we guaranteed to lead with a slash?) and filter will filter out empty components as well. So if you're only and always going to have a leading slash, then splice does the trick. But filter seems like it works for that case and a final slash.

I was likely overly defensive here, worrying aboth leading/trailing slash, I'll check my assumptions. that said, we're pushing cleanURLs to be true by default now when paired with hosting.source, I can probably simplify this code. I'll do that in a follow on so I unblock Next 13 from working

Took on as a TODO to explore simplifying

hassaanp · 2022-10-30T14:17:57Z

when can we expect this to merge?

LorenzoBloedow · 2022-10-31T20:42:35Z

When can we expect the next release with this fix? I tried building it myself a few days ago but had a weird bug where it kept
duplicating my whole project, probably did something wrong in the build process.

jeremyrajan · 2022-11-01T04:54:33Z

i tried the 11.15.0 release, doesnt seem to work tho. Still giving me error that its looking for pages and not app. Anything I am doing wrong?

RajendraAVerma · 2022-11-01T05:35:25Z

i tried the 11.15.0 release, doesnt seem to work tho. Still giving me error that its looking for pages and not app. Anything I am doing wrong?

I tried the 11.15.0 release and hopefully it work 🔥👍 !

 "dependencies": {
    "eslint": "8.26.0",
    "eslint-config-next": "13.0.1",
    "next": "13.0.1",
    "react": "18.2.0",
    "react-dom": "18.2.0"
  }

luisorbaiceta · 2022-11-01T08:20:18Z

i tried the 11.15.0 release, doesnt seem to work tho. Still giving me error that its looking for pages and not app. Anything I am doing wrong?

Version 11.15.0 is from two weeks ago and this has been merged 12 hours ago

JGSolutions · 2022-11-14T13:53:12Z

do u guys still use npm build && npm export?

jamesdaniels · 2022-11-14T22:25:31Z

@JGSolutions ATM we call the Node entry point for Next Build API next/dist/build and we blindly attempt a next export. If the export succeeds we deploy that to the CDN, if it fails we parse the Next Build API and locate all the static artifacts that should be deployed to the CDN. We then use heuristics to determine if a Cloud Functions backend is needed—running a Next Custom Server—when the CDN misses.

In an upcoming release, we're dropping the attempted next export as we've found shortcomings in its assumptions that make it an unreliable early return for building the CDN artifacts. Our own "export" out of the Next Built API is proving just as reliable now that we've built up a good understanding of the manifest files.

sanesanyo · 2023-01-13T09:32:40Z

@jamesdaniels Can you please look at this thread: #5369 ? We have been struggling with Firebase NextJS deployment issues and so far no one has answered from Firebase side. Given you are working on the development, hopefully you can help us out. Thanks a lot in advance.

jamesdaniels added 3 commits October 25, 2022 21:44

WIP of Next 13 fixes

0e5a7e0

Formatting

4db6cb6

gte

578346a

jamesdaniels marked this pull request as ready for review October 26, 2022 17:31

jamesdaniels requested a review from bkendall October 26, 2022 17:31

jamesdaniels mentioned this pull request Oct 26, 2022

[CF3v2 deploy on fresh project] TypeError: Cannot read properties of undefined (reading 'service') #5157

Closed

reowl666 mentioned this pull request Oct 27, 2022

Error: Next.js requires React 18.2.0 to be installed. vercel/next.js#41855

Closed

1 task

This comment was marked as resolved.

Sign in to view

jamesdaniels added 4 commits October 27, 2022 23:36

Bumping deploy test to next 13, adding revalidate and app dir

1e8665d

Enable appdir exp

7037376

linting, semver

de2a50b

Changelog and lint

b6a5ab9

jamesdaniels requested a review from TheIronDev October 28, 2022 03:51

jamesdaniels added 4 commits October 27, 2022 23:57

newline

6853f71

Should be 17 files

e0bdfc1

Unused deps

a4f5e41

20 files actually

8acb5a9

next create --use-npm

7bae042

jamesdaniels added the integration: web frameworks label Oct 28, 2022

jamesdaniels enabled auto-merge (squash) October 28, 2022 16:21

inlined approved these changes Oct 28, 2022

View reviewed changes

jamesdaniels added 2 commits October 31, 2022 15:57

Feedback and todo

f4be89c

Merge branch 'master' into jamesdaniels_next_13

c266bd6

jamesdaniels merged commit 2a675d8 into master Oct 31, 2022

jamesdaniels deleted the jamesdaniels_next_13 branch October 31, 2022 20:14

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Next 13 fixes #5175

Next 13 fixes #5175

jamesdaniels commented Oct 26, 2022 •

edited

codecov-commenter commented Oct 26, 2022 •

edited

This comment was marked as resolved.

jamesdaniels commented Oct 28, 2022

inlined Oct 28, 2022

jamesdaniels Oct 31, 2022 •

edited

inlined Oct 28, 2022

jamesdaniels Oct 31, 2022

jamesdaniels Oct 31, 2022

hassaanp commented Oct 30, 2022

LorenzoBloedow commented Oct 31, 2022

jeremyrajan commented Nov 1, 2022

RajendraAVerma commented Nov 1, 2022 •

edited

luisorbaiceta commented Nov 1, 2022

JGSolutions commented Nov 14, 2022

jamesdaniels commented Nov 14, 2022 •

edited

sanesanyo commented Jan 13, 2023

Next 13 fixes #5175

Next 13 fixes #5175

Conversation

jamesdaniels commented Oct 26, 2022 • edited

codecov-commenter commented Oct 26, 2022 • edited

Codecov Report

This comment was marked as resolved.

jamesdaniels commented Oct 28, 2022

inlined Oct 28, 2022

Choose a reason for hiding this comment

jamesdaniels Oct 31, 2022 • edited

Choose a reason for hiding this comment

inlined Oct 28, 2022

Choose a reason for hiding this comment

jamesdaniels Oct 31, 2022

Choose a reason for hiding this comment

jamesdaniels Oct 31, 2022

Choose a reason for hiding this comment

hassaanp commented Oct 30, 2022

LorenzoBloedow commented Oct 31, 2022

jeremyrajan commented Nov 1, 2022

RajendraAVerma commented Nov 1, 2022 • edited

luisorbaiceta commented Nov 1, 2022

JGSolutions commented Nov 14, 2022

jamesdaniels commented Nov 14, 2022 • edited

sanesanyo commented Jan 13, 2023

jamesdaniels commented Oct 26, 2022 •

edited

codecov-commenter commented Oct 26, 2022 •

edited

jamesdaniels Oct 31, 2022 •

edited

RajendraAVerma commented Nov 1, 2022 •

edited

jamesdaniels commented Nov 14, 2022 •

edited