willmcpo / body-scroll-lock

Body scroll locking that just works with everything 😏
MIT License
4.04k stars 339 forks source link

[3.0.1] Syntax error on IE11 #174

Closed firestar300 closed 4 years ago

firestar300 commented 4 years ago

Hello !

I got this error en Internet Explorer 11 : Syntax error that point to :

const allowTouchMove = el => locks.some(lock => {
...

I use Babel to transpile ES6 code to ES5 with Webpack but it doesn't work for body-scroll-lock part so I don't know if it's an issue with BSL or Babel.

My .babelrc

{
    "presets": [
        ["@babel/preset-env", {
            "targets": {
                "browsers": ["last 2 versions", "ie >= 9", "Firefox ESR"]
            }
        }],
        "preact"
    ],
    "plugins": [
        [
            "@babel/plugin-transform-runtime", {
                "regenerator": true
            }
        ],
        [
            "@babel/plugin-transform-react-jsx", {
                "pragma": "h",
                "pragmaFrag": "Fragment"
            }
        ],
        [
            "@wordpress/babel-plugin-makepot",
            {
                "output": "languages/kmo-app.pot"
            }
        ]
    ],
    "comments": false
}

webpack.config :

{
  entry: config.entry,
  output: {
    path: config.assetsPath,
  },
  externals: {
    jquery: 'jQuery',
  },
  resolve: {
    alias: {
      react: 'preact/compat',
      'react-dom/test-utils': 'preact/test-utils',
      'react-dom': 'preact/compat',
    },
  },
  module: {
    rules: [
      {
        test: /\.js$/,
        exclude: /(node_modules|bower_components)/,
        use: [
          {
            loader: 'babel-loader',
            options: {
              babelrc: true,
            },
          },
          {
            loader: 'eslint-loader',
          },
        ],
      },
      {
        test: /\.css$/,
        use: [
          MiniCssExtractPlugin.loader,
          {
            loader: 'css-loader',
            options: {
              sourceMap: true,
            },
          },
          {
            loader: 'postcss-loader',
            options: {
              sourceMap: true,
              plugins: () => [require('autoprefixer')()],
            },
          },
          'resolve-url-loader',
        ],
      },
      {
        test: /\.(sass|scss)$/,
        use: [
          MiniCssExtractPlugin.loader,
          {
            loader: 'css-loader',
            options: {
              importLoaders: 1,
              url: false,
              sourceMap: true,
            },
          },
          {
            loader: 'postcss-loader',
            options: {
              sourceMap: true,
              plugins: () => [require('autoprefixer')()],
            },
          },
          {
            loader: 'sass-loader',
            options: {
              sourceMap: true,
            },
          },
        ],
      },
    ],
  },
}

Thanks for help.

sectsect commented 4 years ago

@firestar300

Try the following.

webpack.config.js

Solution 1

resolve: {
  mainFields: ['browser', 'main', 'module'],
},

See Doc resolve.mainFields https://webpack.js.org/configuration/resolve/#resolvemainfields

Solution 2

Transpile with Babel

exclude: /node_modules\/(?!(body-scroll-lock)\/).*/,
tu4mo commented 4 years ago

Is there a reason why v3 is not transpiled to ES5 anymore? It's a bit inconvenient having to configure build tools just for this library.

firestar300 commented 4 years ago

Hello @sectsect Thanks for help ! The first solution worked for me.

dlong500 commented 4 years ago

I agree with @tu4mo that the current state of the package is not ideal for people who use tools like webpack. Ideally there would be a straightforward way to include both es5 and native code in a package such that build tools would have automatic options (based on target environments) to: 1) use the transpiled version for a given package 2) transpile the code for a given package

So far we are not even close to being there yet, and have to result to kludgy include/exclude hacks when something breaks. Until this changes, I'd recommend transpiling the primary output of the package to ES5.

sectsect commented 4 years ago

Related: https://github.com/willmcpo/body-scroll-lock/pull/93

willmcpo commented 4 years ago

Hi David,

Thanks for your suggestion.

Would you mind creating a PR capturing this?

On Sun, Apr 26, 2020, 5:27 AM Davison Long notifications@github.com wrote:

I agree with @tu4mo https://github.com/tu4mo that the current state of the package is not ideal for people who use tools like webpack. Ideally there would be a straightforward way to include both es5 and native code in a package such that build tools would have automatic options (based on target environments) to:

  1. use the transpiled version for a given package
  2. transpile the code for a given package

So far we are not even close to being there yet, and have to result to kludgy include/exclude hacks when something breaks. Until this changes, I'd recommend transpiling the primary output of the package to ES5.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/willmcpo/body-scroll-lock/issues/174#issuecomment-619429139, or unsubscribe https://github.com/notifications/unsubscribe-auth/ADJPCOXXYGIOPH7XZNAVFTLROM2QXANCNFSM4MQAYPZQ .

diachedelic commented 4 years ago

@willmcpo I already implemented this in #172

dlong500 commented 4 years ago

I agree that #172 improves the situation, and I guess there's not much more that can be done to make things easier on end users without reverting to only offering the transpiled code (which I agree is also not ideal). I don't know that I would call the situation fixed, however, when it takes tweaking webpack/babel settings to make things actually work. Because webpack prefers the module field over the main field by default it uses the non-transpiled code (which then requires extra manual steps to get babel to transpile code within node_modules). If we change webpack's resolver preferences that might work, though then we risk breaking other things if other packages aren't correctly using the main/module fields. Just ranting a bit here--not really asking for anything at this point. The package management and build ecosystem seems to be a bit of a mess right now when it comes to handling compatibility issues.

diachedelic commented 4 years ago

@dlong500 #172 actually does "only offer transpiled code": the end user can only obtain ES6 code by explicitly importing body-scroll-lock/lib/bodyScrollLock.es6.js. So no webpack configuration is necessary for the user.

dlong500 commented 4 years ago

@diachedelic Sorry, that's not correct. If that were true then my builds wouldn't be broken on IE11. If the module field is present in package.json then webpack uses that by default instead of the main field. Since the module field points to the ES6 code, then that is what webpack is using. Subsequently, babel doesn't transpile the ES6 because code in node_modules isn't transpiled (also standard procedure).

I'm really not trying to be difficult--it simply won't work for older browsers as is unless your build system gets the right files, and webpack won't do that without a bunch of configuration. I suspect there isn't more reporting of the issue in this repo because most people don't deal with IE11 anymore (or aren't using a tool like webpack). Sadly some projects still have to deal with legacy browsers for at least a few more years.

Obviously I would prefer to be using ES6 in general (and not having to transpile to ES5 at all), so I'm glad the ES6 code is included. However, anyone using a build tool and needing to transpile to ES5 for legacy browsers is going to have to do extra work with how things are now (specifically because the vast majority of babel configurations exclude node_modules). And it isn't as simple as just telling babel to transpile ALL node_modules code--that might work but then it will take ages to build the project every time (something the babel folks specifically warn against).

diachedelic commented 4 years ago

@dlong500 #172 has not been merged yet...sorry, didn't mention that! I appreciate your feedback of course. @willmcpo are you ok with me merging #172 ?

sectsect commented 4 years ago

Thanks guys.

@firestar300 v3.0.2 has been released. You no longer need https://github.com/willmcpo/body-scroll-lock/issues/174#issuecomment-619363655 solutions.

firestar300 commented 4 years ago

You are amazing guys ! Thank you all :)