-
Notifications
You must be signed in to change notification settings - Fork 58
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
version 5.1.1 contains an arrow function #99
Comments
@khamiltonuk Thanks for filing this issue! It ended-up being brought-up over on focus-trap/focus-trap#173 and we had a discussion there. Bringing that over to here now: Webpack can be configured to work around this. The issue is you're wanting to use ESM as a modern module system, but still expecting to have old browser-compatible code inside those modules, and not wanting to transpile any third-party modules. Is that correct (just trying to understand the needs)? It feels strange to have diverging targets in the same bundle: ES modules (which is technically ES6) and ES5 JS. But I admit I see the same issue in my bundles that reference Looks like this is a problem for a lot of libraries, and Webpack is keeping its position on prioritizing A lot of packages seem to have solved this by adding a Apparently, this is the browser field spec, but it doesn't really get into specifics about the nature of the referenced files. Still, given its name "browser", it feels like it should point to the UMD build. Thoughts? 👉 I just tried this locally with tabbable's package.json having "browser: dist/index.umd.js" (with Webpack) and the resulting bundle that includes tabbable look nice. Webpack took the right UMD "path" for CJS and plopped the result right into the bundle: }, function(module, exports, __webpack_require__) {
!function(exports) {
"use strict";
var candidateSelectors = [ "input", "select", "textarea", "a[href]", "button", "[tabindex]", "audio[controls]", "video[controls]", '[contenteditable]:not([contenteditable="false"])', "details>summary" ], candidateSelector = candidateSelectors.join(","), matches = "undefined" == typeof Element ? function() {} : Element.prototype.matches || Element.prototype.msMatchesSelector || Element.prototype.webkitMatchesSelector;
function tabbable(el, options) {
var regularTabbables = [], orderedTabbables = [];
return getCandidates(el, (options = options || {}).includeContainer, isNodeMatchingSelectorTabbable).forEach((function(candidate, i) {
var candidateTabindex = getTabindex(candidate);
0 === candidateTabindex ? regularTabbables.push(candidate) : orderedTabbables.push({
documentOrder: i,
tabIndex: candidateTabindex,
node: candidate
});
})), orderedTabbables.sort(sortOrderedTabbables).map((function(a) { // <- NO ARROW!
return a.node;
})).concat(regularTabbables);
}
...
exports.focusable = focusable, exports.isFocusable = isFocusable, exports.isTabbable = isTabbable,
exports.tabbable = tabbable, Object.defineProperty(exports, "__esModule", {
value: !0
});
}(exports); Based on this, I believe the correct solution here is not to transpile ESM into ES5 while still keeping import/export statements that are part of the language syntax of ES6 (diverging targets for the same build), but rather to simply add the |
Fixes #99 AFAICT, this seems to be the best solution to what __a lot__ of folks in the community have been experiencing with ESM bundles, referenced from a package's `module` entry, that aren't also transpiled down to ES5 (which is technically in conflict with ESM, which is an ES6 feature). The problem arises because this package states both `module` and `main`, but Webpack defaults `mainFields` to `['browser', 'module', 'main']`, in order. So it will always pick `module` over `main` by default, because it doesn't understand that the build's target is NOT a browser that supports ESM... See webpack/webpack#5756
Fixes #99 AFAICT, this seems to be the best solution to what __a lot__ of folks in the community have been experiencing with ESM bundles, referenced from a package's `module` entry, that aren't also transpiled down to ES5 (which is technically in conflict with ESM, which is an ES6 feature). The problem arises because this package states both `module` and `main`, but Webpack defaults `mainFields` to `['browser', 'module', 'main']`, in order. So it will always pick `module` over `main` by default, because it doesn't understand that the build's target is NOT a browser that supports ESM... See webpack/webpack#5756
Fixes #99 AFAICT, this seems to be the best solution to what __a lot__ of folks in the community have been experiencing with ESM bundles, referenced from a package's `module` entry, that aren't also transpiled down to ES5 (which is technically in conflict with ESM, which is an ES6 feature). The problem arises because this package states both `module` and `main`, but Webpack defaults `mainFields` to `['browser', 'module', 'main']`, in order. So it will always pick `module` over `main` by default, because it doesn't understand that the build's target is NOT a browser that supports ESM... See webpack/webpack#5756
@all-contributors add @khamiltonuk for bug |
I've put up a pull request to add @khamiltonuk! 🎉 |
Fixes #99 See webpack/webpack#5756 The initial approach was to add a `browser` target to package.json and point it at the UMD build so that bundlers would pick that up instead of the ESM build, since they (except for Rollup) tend to prefer `browser` first, then `module` (ESM), then `main`. But that would potentially affect tree-shaking, and we don't want to hamper that optimizing process. So this PR transpiles the code in the ESM bundle down to ES5, so that it runs in IE9+ like the CJS and UMD bundles do, while keeping the import/export ESM statements.
OK, folks, this is hopefully finally fixed in |
I am trying to use the library in internet explorer 11
in the transpiled version there is an arrow function
I can see it coming from line 49
https://github.com/focus-trap/tabbable/blob/master/src/index.js#L49
We have two options, we can use a traditional function expression in the
index.js
, or we can configure the babel settings to have a target of internet explorer 9 to match the version support in the docsThe text was updated successfully, but these errors were encountered: