hapijs / nes

WebSocket adapter plugin for hapi routes
Other
502 stars 87 forks source link

Fails to load in IE11 due to ES6 syntax #296

Closed iancamp closed 4 years ago

iancamp commented 4 years ago

Support plan

Context

What are you trying to achieve or the steps to reproduce?

import *  as Nes from '@hapi/nes/lib/client'

What was the result you got?

Syntax error in vendor.js due to line 30 of client.js containing an arrow function.

What result did you expect?

The page to load correctly and for Nes to successfully connect / send requests to the server.

--

Replacing all arrow functions with function (...) {...} and shorthand style properties with prop: prop in JSON objects results in the page loading correctly. However, due to change in scope when using function(...), this is incorrectly used in most cases and results in an error when calling socket.connect(). I stopped short of adding an outer var _this = this; variable and updating references since I'm not sure if that's the preferred solution.

hueniverse commented 4 years ago

You have to use webpack or something like it to adjust the code to the platforms you need to support. There is no one right answer here.

ceisele-r commented 4 years ago

@hueniverse we are experiencing the same error after updating from nes 7.0.3 to @hapi/nes 11.2.2 together with updating hapi from 18.1.0 to @hapi/hapi 18.4.0 and inert from 5.1.0 to @hapi/inert 5.2.2. We have been using webpack before and after the update. Before, everything works in IE11 but after the update, the Arrow functions unfortunately are not transpiled.

Do you have any idea why this could be happening now after the update? One thing we did was to apply the suggested fix mentioned in https://github.com/hapijs/nes/issues/294#issuecomment-538705598 because we ran in that issue. Could it be somehow related to this?

iancamp commented 4 years ago

I was able to get around this by adding babel-loader. See below.

Packages installed

    "@babel/core": "7.7.2",
    "@babel/polyfill": "7.7.0",
    "@babel/preset-env": "7.7.1",
    "babel-loader": "8.0.6"

babel.config.js

'use strict';

module.exports = function (api) {
    api.cache(true);

    const presets = [ '@babel/preset-env'];
    const plugins = [];

    return {
        presets,
        plugins
    };
}; 

Added this to my webpack.config.js

    module: {
        rules: [
            {
                test: /\.js$/,
                include: [ path.resolve('./node_modules/@hapi/nes') ],
                use: {
                    loader: 'babel-loader',
                    options: {
                        configFile: path.resolve('./babel.config.js')
                    }
                }
            }
        ]
    },
ceisele-r commented 4 years ago

@iancamp thank you. We have a similar setup, except that we do not use @babel/polyfill since it is deprecated.

I think I now proceeded some steps by adding @babel/plugin-transform-arrow-functions and including it as plugin to babel-loader:

use: {
          loader: "babel-loader",
          options: {
            presets: [
              [
                "@babel/preset-env",
                {
                  "useBuiltIns": "usage",
                  "corejs": 3,
                  "debug": true,
                  "targets": {
                    "chrome": "58",
                    "ie": "11"
                  }
                }
              ],
              ["@babel/preset-react"]
            ],
            plugins: [
              ...
              "@babel/plugin-transform-runtime",
              "@babel/plugin-transform-arrow-functions"
            ],
          }
        }

I am wondering whether adding @babel/plugin-transform-arrow-functions should be necessary or has any downsides. At least it seems to somehow be working like this in IE11 again at the first look. I still have to see if it really works since I did quite some changes around this when trying to solve the issue and therefore a lot of stuff is mixed together now.

iancamp commented 4 years ago

Oh, didn't realize @babel/polyfill is deprecated. Thanks for the heads up! I don't think it's necessary for the workaround, anyway.