Skip to content
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

react-intl isn't recognized as ES6 module #48

Open
httpiga opened this issue Jul 23, 2020 · 5 comments
Open

react-intl isn't recognized as ES6 module #48

httpiga opened this issue Jul 23, 2020 · 5 comments
Labels
Hacktoberfest Hacktoberfest 2021

Comments

@httpiga
Copy link

httpiga commented Jul 23, 2020

After a lot of headaches, I figured out that are-you-es5 doesn't recognize react-intl as non-ES5 module.
I am using next-transpile-modules to transpile modules reported by are-you-es5 as non-ES5 but, after checking webpack chunks, I realized that they weren't 100% ES5 compliant but there was still a few of arrow functions (since arrow functions were introduced in ES6 I used their presence as a test).
After a few of trial and error, aka adding random modules to next-transpile-modules module list, I found that that arrow functions were introduced by react-intl.
After reading this thread I think that the testing method used by are-you-es5 to check whether a module is ES5 may be weak, as it (at least) doesn't check for the presence of arrow functions in the module.

@obahareth
Copy link
Owner

Hey @nicolaelia, sorry for the headache you faced. There's a lot of possible cases with libraries and this sadly is one of the cases I hoped wouldn't happen. The checker itself is using acorn which would detect the arrow functions without issues, but for performance reasons we limited the checking to only happen on the entrypoint script (in this case index.js) assuming that if the library is ES6+ then the entrypoint script would be as well.

Here's the index.js from react-intl:

"use strict";
function __export(m) {
    for (var p in m) if (!exports.hasOwnProperty(p)) exports[p] = m[p];
}
Object.defineProperty(exports, "__esModule", { value: true });
var createFormattedComponent_1 = require("./src/components/createFormattedComponent");
function defineMessages(msgs) {
    return msgs;
}
exports.defineMessages = defineMessages;
function defineMessage(msg) {
    return msg;
}
exports.defineMessage = defineMessage;
var injectIntl_1 = require("./src/components/injectIntl");
exports.injectIntl = injectIntl_1.default;
exports.RawIntlProvider = injectIntl_1.Provider;
exports.IntlContext = injectIntl_1.Context;
var useIntl_1 = require("./src/components/useIntl");
exports.useIntl = useIntl_1.default;
var provider_1 = require("./src/components/provider");
exports.IntlProvider = provider_1.default;
exports.createIntl = provider_1.createIntl;
// IMPORTANT: Explicit here to prevent api-extractor from outputing `import('./src/types').CustomFormatConfig`
exports.FormattedDate = createFormattedComponent_1.createFormattedComponent('formatDate');
exports.FormattedTime = createFormattedComponent_1.createFormattedComponent('formatTime');
exports.FormattedNumber = createFormattedComponent_1.createFormattedComponent('formatNumber');
exports.FormattedList = createFormattedComponent_1.createFormattedComponent('formatList');
exports.FormattedDisplayName = createFormattedComponent_1.createFormattedComponent('formatDisplayName');
exports.FormattedDateParts = createFormattedComponent_1.createFormattedDateTimePartsComponent('formatDate');
exports.FormattedTimeParts = createFormattedComponent_1.createFormattedDateTimePartsComponent('formatTime');
var createFormattedComponent_2 = require("./src/components/createFormattedComponent");
exports.FormattedNumberParts = createFormattedComponent_2.FormattedNumberParts;
var relative_1 = require("./src/components/relative");
exports.FormattedRelativeTime = relative_1.default;
var plural_1 = require("./src/components/plural");
exports.FormattedPlural = plural_1.default;
var message_1 = require("./src/components/message");
exports.FormattedMessage = message_1.default;
var utils_1 = require("./src/utils");
exports.createIntlCache = utils_1.createIntlCache;
__export(require("./src/error"));

I think this is a very valid case for implementing #2 (also relevant discussion in #24), but that's something I need help with because I suspect it would have a huge performance hit if not done properly.

However I am a little confused, I installed react-intl locally and I don't see arrow functions in the index file or files it imports, is there a specific version this is happening on?

@httpiga
Copy link
Author

httpiga commented Jul 24, 2020

Hello @obahareth , thank you for your reply.
The react-intl version which I'm using is 3.12.1 and the arrow functions I'm talking about are located in some files in the ./lib folder of the package, but not in the index.js file (obviously 🙄).
This comment in the react-intl issue I've mentioned above explains why I got that ES6 code in my bundle.

It being understood that this is more a react-intl bug than a are-you-es5 one, nevertheless I still think that assuming that if index.js of a package is ES5 then all the package must be ES5, makes this library weak and checking the entire package should be the most robust way.
I see the performance implications which come by testing the entire package instead of just index.js, but I can point two observations:
You can stop checking as soon as you find a non-es5 file inside the package, to avoid unnecessary further testing, and since the common workflow is running are-you-es5 in the postinstall, so it isn't a frequent execution, a few-seconds increase in the execution time could be an acceptable deal for a 100% confidence in the results.

@obahareth
Copy link
Owner

obahareth commented Jul 25, 2020

Hey @nicolaelia, I agree with you and that's definitely one of the steps we were planning on tackling, with a configurable depth for how deep a level of imports we should check as @tooppaaa suggested before. However I think even doing that wouldn't help with anything because we are checking for the "main" attribute to determine which is the index file, and in that case all of react-intl is ES5. I think implementing #45 is part of the way to solve this, but I'm not sure on how to work with the "module" entry point (which react-intl is using) since it's still not officially part of package.json.

It seems that bundlers always prefer the module entry point if it's there, and that's already a sign that a package is ES6+ so perhaps we can just keep it at that, if the module entry point exists then we consider it ES6?

@httpiga httpiga changed the title Doesn't recognize react-intl as ES6 module react-intl isn't recognized as ES6 module Jul 29, 2020
@httpiga
Copy link
Author

httpiga commented Jul 29, 2020

It makes sense to me. Since the "module" in package.json isn't official yet, I would add a flag to enable this function, or at least it should be mentioned in the readme.md.
Thank you for your time, it is greatly appreciated!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Hacktoberfest Hacktoberfest 2021
Projects
None yet
Development

No branches or pull requests

3 participants
@obahareth @httpiga and others