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

Expose app.js #4129

Merged
merged 16 commits into from Apr 12, 2018
Merged

Expose app.js #4129

merged 16 commits into from Apr 12, 2018

Conversation

timneutkens
Copy link
Member

@timneutkens timneutkens commented Apr 9, 2018

Fixes #88
Fixes #4029

Examples will be added when this PR lands, since this PR involves a lot of changes I didn't want to mess up the diff with unrelated files

  • Is rendered on both client and server side
  • Markup can be persisted between page changes
  • this.state should not be overwritten. Instead create a new component to handle the state (layout example)
  • Handles calling getInitialProps of the page Component
  • Implements componentDidCatch on the client side, and renders the error page accordingly. (overriding componentDidCatch example)
  • The Container implements React Hot Loader when in development mode.
  • The Container implements scrolling to a hash when using <Link href="/about#example-hash">

To override the default _app.js behaviour you can create a file ./pages/_app.js, where you can extend the App class:

import App, {Container} from 'next/app'
import React from 'react'

export default class MyApp extends App {
  static async getInitialProps ({ Component, ctx }) {
    let pageProps = {}

    if (Component.getInitialProps) {
      pageProps = await Component.getInitialProps(ctx)
    }

    return {pageProps}
  }

  render () {
    const {Component, pageProps} = this.props
    return <Container>
      <Component {...pageProps} />
    </Container>
  }
}

import { makePublicRouterInstance } from './router'

export default class App extends Component {
state = {
hasError: null
}

static displayName = 'App'

static async getInitialProps ({ Component, router, ctx }) {
Copy link
Contributor

Choose a reason for hiding this comment

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

If user didn't override getInitialProps in his/her sub class, is this going to execute?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yep 👍 (Basically the same as _document.js that also implements getInitialProps

lib/app.js Outdated
pathname: router.pathname,
asPath: router.asPath,
get query () {
warn(`Warning: 'url.query' is deprecated. https://err.sh/next.js/url-deprecated`)
Copy link
Contributor

Choose a reason for hiding this comment

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

Shall we make sure we only print this once?

Copy link
Member Author

Choose a reason for hiding this comment

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

Done 👍

@@ -17,7 +17,7 @@ export async function getPagePaths (dir, {dev, isServer, pageExtensions}) {

if (dev) {
Copy link
Contributor

Choose a reason for hiding this comment

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

We need to add app.js to the comment :)

Copy link
Member Author

Choose a reason for hiding this comment

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

Done 🔑


export default class MyDocument extends Document {
static getInitialProps ({ renderPage }) {
const { html, head, errorHtml, chunks } = renderPage()
Copy link
Contributor

@arunoda arunoda Apr 9, 2018

Choose a reason for hiding this comment

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

Shall we get these from the Document. Like this:

static getInitialProps(info) {
  const initialProps = Document.getInitialProps(info);
  return {
    ...initialProps,
    customProperty: 'Hello World'
  } 
}

Then it's very simple and we don't need expose internal things like renderPage. And let's make sure we update this for _app.js and inside README as well.

@arunoda
Copy link
Contributor

arunoda commented Apr 9, 2018

Looks pretty nice to me.

@corysimmons
Copy link
Contributor

Just confirming this is the PR to watch for page transition support?

@timneutkens
Copy link
Member Author

timneutkens commented Apr 9, 2018

@corysimmons it's a lower level API that could be used to implement a lot of different things.

lib/app.js Outdated
back: () => {
warn(`Warning: 'url.back()' is deprecated. Use "window.history.back()"`)
warn(`Warning: 'url.back()' is deprecated. Use "window.history.back()" https://err.sh/next.js/url-deprecated`)
Copy link
Contributor

@arunoda arunoda Apr 10, 2018

Choose a reason for hiding this comment

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

Don't we use something like warnOnce or something. And for the rest of the places.

Copy link
Member Author

Choose a reason for hiding this comment

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

I believe this was deprecated in v2 or v3. That's what I didn't want to touch it too much, just adding the url to the message. In theory we could use the same message though. Will update it.

readme.md Outdated
</details></p>

- Is rendered on both client and server side
- `this.state` should not be overwritten. Instead create a new component to handle the state ([layout example](./examples/layout-component))
Copy link
Contributor

Choose a reason for hiding this comment

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

This is not clear. What are we going to say.

Copy link
Member Author

Choose a reason for hiding this comment

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

Added a clearer example.

readme.md Outdated
- Is rendered on both client and server side
- `this.state` should not be overwritten. Instead create a new component to handle the state ([layout example](./examples/layout-component))
- Handles calling `getInitialProps` of the **page** `Component`
- Implements `componentDidCatch` on the client side, and renders the error page accordingly. ([overriding `componentDidCatch` example](./examples/componentdidcatch))
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's try not say what've inside app.js. But what the user can do with this. Here's a sample:

Next.js has a App layout which initialize pages in your app. You can override it with pages/_app.js and implement:

  • Layouts works across pages
  • Animations in between pages
  • Keep the DOM state when navigating page
  • Provide additional properties to initial props of pages
  • Custom error messages by catching errors
    ...

Copy link
Member Author

Choose a reason for hiding this comment

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

Updated

Copy link
Contributor

@arunoda arunoda left a comment

Choose a reason for hiding this comment

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

Added some suggestions for docs.
May contain grammar errors :D

readme.md Outdated
<ul><li><a href="./examples/componentdidcatch">Using `_app.js` to override `componentDidCatch`</a></li></ul>
</details></p>

Next.js uses the `App` component to initialize pages. This can be used for:
Copy link
Contributor

Choose a reason for hiding this comment

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

This convey the message better:

Next.js uses the App component to initialize pages. You can override it and control the page initialization. With that you can do amazing things like:

readme.md Outdated
- Persisting layout between page changes
- Keeping state when navigating pages
- Custom error handling using `componentDidCatch`
- Handling of calling **page** `Component`s `getInitialProps`
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a good example for getInitialProps:

  • Inject additional data into pages (like by processing GraphQL queries)

readme.md Outdated
- Custom error handling using `componentDidCatch`
- Handling of calling **page** `Component`s `getInitialProps`

To override the default `_app.js` behaviour you can create a file `./pages/_app.js`, where you can extend the `App` class:
Copy link
Contributor

Choose a reason for hiding this comment

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

To override, create a file in ./pages/_app.js and override the App class as shown below:

@PetrSnobelt
Copy link
Contributor

Is it possible to have multiple layouts using this feature?
It looks like the only way is to use if statement in _app.js which can be difficult.
Will be fine if a page can specify which app/layout would like to use.

@timneutkens
Copy link
Member Author

timneutkens commented Apr 10, 2018

It looks like the only way is to use if statement in _app.js which can be difficult.

This is definitely not the way to go. Since you'd lose code splitting and other advantaged. What you could do however is this:

@tz5514 made a PR implementing a custom renderPage api. The change he made actually involved only 1 change in the App component. So his idea can be implemented by the community based on this pull request. The specific PR is: #3552

@arunoda
Copy link
Contributor

arunoda commented Apr 10, 2018

@PetrSnobelt

It looks like the only way is to use if statement in _app.js which can be difficult.

I don't find using if is difficult. Usually, that's the way you can show different content in React apps.
I am not just talking about if, but all the basic JS control structures.

Will be fine if a page can specify which app/layout would like to use.

Yes you can do that. You can mention the layout name in your page like this:

const MyPage = () => (
  <div>This is my page</div>
)

MyPage.layout = "main-layout"

export default MyPage

Then you can read the layout field inside the render function of _app.js and use the correct layout.
You can even use our dynamic import support to avoid loading all the layouts at once.

Copy link
Contributor

@arunoda arunoda left a comment

Choose a reason for hiding this comment

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

This is looks great.

@tz5514
Copy link

tz5514 commented Apr 11, 2018

@timneutkens There is a missing item Custom <App> in readme.md navigation menu,
beween Dynamic Import and Custom <Document>:
image

@timneutkens
Copy link
Member Author

@tz5514 great catch, I believe the table of contents hasn't been updated for a while, I'll udpate it right now 👍

@timneutkens
Copy link
Member Author

Fixed the table of contents ✅

@PetrSnobelt
Copy link
Contributor

PetrSnobelt commented Apr 11, 2018

@timneutkens, @aronuda That makes sense, thanks
I like #3461 idea and hope it can be used with _app.js

@timneutkens
Copy link
Member Author

@PetrSnobelt As you can see here: https://github.com/zeit/next.js/pull/3461/files#diff-c39f41208533f29db9e4150b6efd2b68

The change involves changing app.js, so yeah, definitely possible 😄

# Conflicts:
#	readme.md
#	server/render.js
@pozylon
Copy link

pozylon commented Apr 12, 2018

Absolutely amazing work @timneutkens. Thank you so much for bringing us this, I can implement about 5 feature requests the way it should be done for various customers after this gets merged.

@timneutkens timneutkens merged commit eca8e8f into vercel:canary Apr 12, 2018
@timneutkens timneutkens deleted the add/layout branch April 12, 2018 08:33
@tz5514
Copy link

tz5514 commented Apr 12, 2018

Finally! Huge thanks for @timneutkens and core team. Now I can release next-layouts in fews days. Great work, thanks!

@willmeierart
Copy link
Contributor

So, will we have to wait for 6 to use this feature? Thank you soooo much for opening this door for us!

@timneutkens
Copy link
Member Author

timneutkens commented Apr 15, 2018

It'll be out on canary when another PR I'm working on right now gets merged (will be opened on the repository today 👍

@tim-phillips
Copy link
Contributor

6.0.0-canary.4 is released

@lock lock bot locked as resolved and limited conversation to collaborators Apr 19, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
9 participants