insin / nwb

A toolkit for React, Preact, Inferno & vanilla JS apps, React libraries and other npm modules for the web, with no configuration (until you need it)
Other
5.57k stars 332 forks source link

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

Closed Racle closed 7 years ago

Racle commented 7 years ago

This issue is a:

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 commented 7 years ago

πŸ‘ + 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 commented 7 years ago

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 commented 7 years ago

@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 commented 7 years ago

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 https://github.com/facebookincubator/create-react-app/pull/2304 too, as Dan Abramov has the hot reloading bug again 😸

xzilja commented 7 years ago

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 commented 7 years ago

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 commented 7 years ago

Thanks, will have a look

insin commented 7 years ago

@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 😹

xzilja commented 7 years ago

@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.