IE11 not working with latest release

[bparish628]

The issue here is that a new feature was added in v6 that does not include ie11 supported code. The culprit is the because the userOptions is being spread onto an object.
https://github.com/focus-trap/focus-trap/blob/master/index.js#L48

Introduced in #125

Does this code get transpiled or should the src be compliant? I'd be happy to make a fix to it, just want to verify what the intention is!

Error is Expected identifier, string or number
@ this line

Steps to Recreate

• Update to v6
• Run in ie11 and see js errors

[stefcameron]

@bparish628 👋 Thanks for reaching out! We definitely transpile the code (and those settings, without any browser targets, should be putting everything back down the ES5) -- at least, it's supposed to be getting transpiled!

However, looking at ./dist/focus-trap.js in the published package, I can see that it's in fact not the case, which isn't the intent.

I'll try to fix this ASAP!

[stefcameron]

Problem seems to be that @rollup/plugin-babel is ignoring babel.config.js when the build script runs. Compiling babel index.js produces properly transpiled code that would be ES5-compatible.

[stefcameron]

Unfortunately, I've been trying to get rollup (not a fan!!) to use Babel for the past hour and a half, trying everything I can think of, nothing is working. @rollup/plugin-babel does read the config (because it throws an error and fails the build if I include an invalid config option Babel doesn't know about), but does absolutely nothing with it. The code never gets transpiled. If I run babel index.js, as I mentioned before, I get the desired output. So the issue is with @rollup/plugin-babel... Any ideas?

[stefcameron]

Of course, no one on the Internet seems to have this issue. The plugin just works...

[bparish628]

@stefcameron
Just a thought, but could it be because it's running babel after terser minifies/mangles it? I'd be curious if that could cause babel to not transpile it. I'm not super familiar with rollup, so take that with a grain of salt!

[stefcameron]

@bparish628 Thanks for the suggestion. Could be plugin order. I tried running the babel plugin as the first one in the plugin list, didn't change a thing. I can try running it after terser() to see.

One thing I do see -- and maybe this supports your theory -- is that the minified files seem to be transpiled. Can you give focus-trap.min.js a try to see if you have a problem in IE11? I think it'll work.

[stefcameron]

Unfortunately, changing the order of plugins to execute babel() after terser() doesn't change anything for the non-minified bundles.

[bparish628]

If I use the minified version, it looks to get past the issue in focus-trap. However, it then fails on tabbable not being compliant 😢.

[bparish628]

I was looking at the rollup config, and I don't think it's running the non-minified through babel. https://github.com/focus-trap/focus-trap/blob/master/rollup.config.js#L92

Wouldn't you would want to add plugins: [babel(...)] or plugins: commonPlugins to run it through that?

[stefcameron]

@bparish628 Oh no, don't tell me... 🤦 Yep, that's it. Thank you for being my second set of 👀 . Just couldn't see it!

I think there's another issue with babel in that the ESM bundle should be created with ESM browsers as a target, not ES5 browsers, so I'll take the opportunity to make sure that happens correctly as well while I'm in there.

[stefcameron]

@bparish628 Should be fixed in #173. Can you have a look, see if it looks good to you?

[stefcameron]

If I use the minified version, it looks to get past the issue in focus-trap. However, it then fails on tabbable not being compliant 😢.

Thanks for checking that. tabbable has the same build system, so the same issue. I'll have to fix it there as well.

[stefcameron]

@all-contributors add @bparish628 for bug

[allcontributors]

@stefcameron

I've put up a pull request to add @bparish628! 🎉

[stefcameron]

Published in focus-trap@6.1.2

[stefcameron]

Tabbable was fixed in 5.1.1

[bparish628]

@stefcameron Sorry for commenting on a closed ticket, but I was testing the latest version and it looks like tabbable still doesn't work as expected 🤔 (it's using 5.1.1). I was looking at the dist and the index.esm has

var tabbableNodes = orderedTabbables.sort(sortOrderedTabbables).map(a => a.node).concat(regularTabbables);

while the index.js has

  var tabbableNodes = orderedTabbables.sort(sortOrderedTabbables).map(function (a) {
    return a.node;
  }).concat(regularTabbables);

Error:

This library is working awesome though! No errors from here, just from tabbable

[stefcameron]

No worries!

@bparish628, @khamiltonuk has the same inquiry over on #173 just now, my response there. Let's continue the discussion there to keep it one place.

[stefcameron]

I'm re-opening this on account of focus-trap/tabbable#99 and to track that we need to add the browser: ./dist/index.umd.js entry to package.json for focus-trap to avoid this in the future since focus-trap targets ESM browsers for its ESM build, just as tabbable does. This seems to be how the majority of other packages have worked around the same issue based on webpack/webpack#5756

[stefcameron]

I have published focus-trap@6.1.3 that bumps tabbable to 5.1.2, so this should be fixed all the way down.

[bparish628]

@stefcameron Awesome work! I have pulled down the latest and everything appears to be working as I would expect 💯. Thanks for the quick turnaround on this issue. 😄

[stefcameron]

Wonderful, glad to hear it's working. My pleasure!