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

hot-reload doesn't work with nwb/express #287

Closed
1 of 4 tasks
Racle opened this issue Mar 28, 2017 · 9 comments
Closed
1 of 4 tasks

hot-reload doesn't work with nwb/express #287

Racle opened this issue Mar 28, 2017 · 9 comments
Labels

Comments

@Racle
Copy link

Racle commented Mar 28, 2017

This issue is a:

  • Bug report
  • Feature request
  • Question / support request
  • Other

System:
Windows 10 enterprise
Node 6

Installed modules:

  "dependencies": {
    "express": "^4.15.2",
    "material-ui": "^0.17.1",
    "react": "^15.4.2",
    "react-dom": "^15.4.2",
    "react-flexbox-grid": "^1.0.2",
    "react-router": "^4.0.0",
    "react-router-dom": "^4.0.0",
    "react-tap-event-plugin": "^2.0.1"
  },
  "devDependencies": {
    "nwb": "^0.15.6",
    "nwb-sass": "^0.7.1"
  },

Description:

Got basic app structure (created with nwb new react-app), with added server folder in root. Server folder includes index.js. Hot reload doesn't work with every change. ex. adding h2 tag with text doesn't seems to be working (or changing text), but adding Row or Col, it's seems to be changing web page.
server/index.js file changes doesn't register to reload.
Console says next:

Recompiling...
Compiled successfully in 142 ms.

And in chrome console

[HMR] bundle rebuilt in 277ms

On network tab there is no sign of hot reload.

Nothing happens on localhost:3000, but after restarting node server/index.js, all my changes are there.
index.js

const express = require('express');

const app = express();

app.use(require('nwb/express')(express))

app.use(express.static('public'))


app.get('/api/hello', function (req, res)  {
  res.send('hello world');
})


app.listen(3000, (err) => {
  if (err) {
    console.error('error starting server:')
    console.error(err.stack)
    process.exit(1)
  }
  console.log('server listening at http://localhost:3000')
})

I start server with node server/server.js (script in my package.json)

@xzilja
Copy link

xzilja commented May 2, 2017

👍 + 1, same issue on osx and chrome (not tested elsewhere), terminal and browser both log change events yet nothin on page reloads / changes. Using reload option like app.use(require("nwb/express")(express, { reload: true })); doesn't seem to work either.

This happens in fresh nwb react build using hello world example.

@insin
Copy link
Owner

insin commented May 30, 2017

Can you provide a repro repo so this is easier to test?

I use the middleware for hot-reloading development at work with no problems, so a full repo allows me more easily compare a working configuration with a broken one.

@xzilja
Copy link

xzilja commented May 30, 2017

@insin can't provide full repo as it is private, but here is server.js that uses the middleware:

const express = require("express");
const compression = require("compression");
const app = express();
const nwb = require("nwb/express");

// Disable X-Powered-By header for security reasons
app.disable("x-powered-by");

// Set port
app.set("port", process.env.PORT || 3000);

// Production specific config
if (process.env.NODE_ENV === "production") {
  // gZip
  app.use(compression());

  app.use(express.static("dist"));

  app.get("*", (req, res) => {
    res.sendFile(`${__dirname}/dist/index.html`);
  });
} else {
  app.use(nwb(express));
}

// Start the app
app.listen(app.get("port"), error => {
  if (error) return console.error(error.message);
  console.log(`🚨 Server started at: port: ${app.get("port")}`);
  console.log(`🍃 Enviroment: ${process.env.NODE_ENV}\n`);
});

let me know if there are other places you need to examine, will do my best to post snippets here. In general hot reloading appears to work, i.e. browser receives and logs events in console, there are outputs in terminal, yet nothing changes visually on a page until manually refreshed.

@insin
Copy link
Owner

insin commented May 30, 2017

A sample of your client app's entry point and how you write components would be useful too, in that case, as nwb is still using (the now-deprecated) https://github.com/gaearon/react-transform-hmr

Keeping a keen eye on facebook/create-react-app#2304 too, as Dan Abramov has the hot reloading bug again 😸

@xzilja
Copy link

xzilja commented May 30, 2017

Sure, here they are:

App.js

import React, { Component } from "react";
import PushNotificationIOS from "./components/PushNotificationIOS";

class App extends Component {
  render() {
    return <PushNotificationIOS />;
  }
}

export default App;

PushNotificationIOS.js

import React, { Component } from "react";
import styled, { keyframes } from "styled-components";
import anchor from "../assets/anchor.svg";

const slide = keyframes`
  0% { opacity: 0; transform: translateY(-20px); }
  100% { opacity: 1; transform: translateY(0); }
`;

const SContainer = styled.div`
  width: 300px;
  border-radius: 15px;
  background-color: rgba(236, 236, 236, 0.9);
  overflow: hidden;
  box-shadow: 0 1px 2px 0 rgba(0, 0, 0, 0.3);
  color: #424242;
  opacity: 0;
  transform: translateY(-40px);
  will-change: transform, opacity;
  animation: .4s ease .3s forwards ${slide};
`;

const SHeader = styled.div`
  background-color: rgba(255, 255, 255, 0.85);
  padding: 6px 33px;
  letter-spacing: 1px;
  position: relative;

  &::before {
    content: '';
    width: 18px;
    height: 18px;
    background-color: #4db7c3;
    background-image: url(${anchor});
    background-size: auto 80%;
    background-position: center;
    display: block;
    position: absolute;
    left: 9px;
    top: 7px;
    border-radius: 3px;
  }

  &::after {
    content: "now";
    position: absolute;
    display: inline-block;
    right: 12px;
    font-size: 10px;
    opacity: 0.8;
    top: 9px;
  }
`;

const SContent = styled.div`
  padding: 10px 15px;
  opacity: 0.8;
  font-size: 14px;
`;

const SFooter = styled.div`
  font-size: 10px;
  opacity: 0.8;
  padding: 0 15px 7px 15px;
`;

class PushNotificationIOS extends Component {
  render() {
    return (
      <SContainer>
        <SHeader>Loot</SHeader>
        <SContent>€7.80 ~ £6.20 Spent</SContent>
        <SFooter>Press for more</SFooter>
      </SContainer>
    );
  }
}

export default PushNotificationIOS;

@Racle
Copy link
Author

Racle commented May 30, 2017

Here is full test app. Server and client side code. Just run yarn install or npm install and then npm run start-server. Might have some leftovers, its stripped down from what i've used.

https://lonke.ro/tmp/nwb_test.zip

@insin
Copy link
Owner

insin commented May 30, 2017

Thanks, will have a look

@insin
Copy link
Owner

insin commented Jun 2, 2017

@Racle - it's not patching the component because you're defining render like this:

render = () => {

I don't think React Transform HMR knows how to deal with this, or isn't looking for this way of defining it, as you don't need to bind render to the instance.

It works if you define render like this instead:

render() {

Something else I noticed based on a console error - you don't need the <script src="index.js"></script> in your index.html, as HtmlWebpackPlugin handles inserting <script> tags to load the app.

You also might not need app.use(express.static('public')) when using nwb's middleware, as CopyPlugin should be serving the contents of public/ from memory when that directory exists.

I'm totally stealing that // THIS IS POTENTIALLY OK comment for future use, too 😹

@insin insin closed this as completed Jun 2, 2017
@xzilja
Copy link

xzilja commented Jun 2, 2017

@insin as a side note, my components are standard react classes with normal render i.e.

class Button extends Component {
  static propTypes = {
    children: PropTypes.string.isRequired,
    onClick: PropTypes.func.isRequired
  };

  render() {
    const { children, onClick, ...props } = this.props;
    return (
      <button onClick={onClick} {...props}>
        {children}
      </button>
    );
  }
}

And development is not using static

const express = require("express");
const compression = require("compression");

const app = express();

// Disable X-Powered-By header for security reasons
app.disable("x-powered-by");

// Set port
app.set("port", process.env.PORT || 3000);

// Production specific config
if (process.env.NODE_ENV === "production") {
  // gZip
  app.use(compression());

  app.use(express.static("dist"));

  app.get("*", (req, res) => {
    res.sendFile(`${__dirname}/dist/index.html`);
  });
} else {
  const nwb = require("nwb/express");
  app.use(nwb(express));
}

// Start the app
app.listen(app.get("port"), error => {
  /* eslint-disable */
  if (error) return console.error(error.message);
  console.log(`🚨 Server started at: port: ${app.get("port")}`);
  console.log(`🍃 Enviroment: ${process.env.NODE_ENV}\n`);
});

So I am still confused to why any sort of reloading won't work. Again all events seem to be received fine in console, just nothing updates on screen. I wonder if react 16 could be an issue? 🤔 Although I am not using nothing fancy like returning components in array etc.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants