ES6 Module in browser
Closed this issue · 13 comments
gettext.js throws an exception and is inaccessible when loaded as an ES 6 module in a browser.
I develop my stuff in browsers that support modules and then pack the code. I did not check the packed stuff yet, but for dev-mode gettext.js works when when passing 'window' in stead of 'this' in the global wrapper function (last line of gettext.js). I guess in node 'window || this' would throw an 'undefined' error, right? So something more elaborate would need to be done.
Please let me know, if you would like me to work something out (browser dev and packed and node).
Cheers and thanks for the lib!
Hi @schrotie,
You're right, I'd be glad if you could look into it and make a small PR that fixes that. I wasn't coding in ES6 back then when I created gettext.js and my current projects still use it with require.js.
Best
So I read up on how this could be done. Trouble is, until node and every browser fully supports ESM (i.e. not in the foreseable future)
export default i18n;
or some such thing will be invalid syntax. window will be undefined in node (though that would be catchable). This person thinks, you should build to solve this:
https://thedrearlight.com/blog/delivering-js-modules.html
Since you already have a gulp file, I think this approach would make sense. May require some more gulp dev-deps. But if done right I think it would be relatively straightforward to have separate builds for various module systems while keeping the code clean (I hate those parts where libs try some arcane magic live to try to figure out how to export there functionality to me and then break :-) ).
I tend to go checking how rollup could help because it has output support for amd, cjs, esm, iife, system, and umd, and works well with gulp. What do you think?
Hi @schrotie,
Thanks for the reading, the blog post you linked is indeed informative.
I tend to go checking how rollup could help because it has output support for amd, cjs, esm, iife, system, and umd, and works well with gulp. What do you think?
Rollup seems a good choice for gettext tiny lib. Various outputs seems nice. If you have the time and the desire to look at it, it would be awesome :)
Best
Heyho @guillaumepotier
So I made good on my threats and used rollup to produce various module formats ... 7 to be precise ... times two for minified versions ... 14
Looks a bit menacing to be honest :-) I just dumped every format that rollup can do. Plus the global definition on window that you had there. It might suffice to provide amd, cjs, esm, and iife out of the box and provide gulp tasks for the others.
Since rollup likes to ingest esm I changed the source to that - removed the wrapper function, added a default export and changed the indentation. I may be able to get it working with another format. As it is the tests would need to use a build instead of the source. And I might add a bit of testing for some builds.
Please let me know what you think about the proposed changes before I invest more time. You can find it at https://github.com/schrotie/gettext.js
Cheers
Thorsten
Hi @schrotie,
Indeed this might look menacing but I'm not afraid ;)
It might suffice to provide amd, cjs, esm, and iife out of the box and provide gulp tasks for the others.
Indeed, it seems unneeded to dump every supported Rollup output format, amd, cjs, esm should be enough.
As it is the tests would need to use a build instead of the source.
Indeed, I cannot merge something with broken or erased tests ;)
Feel free to add more time on your fork, I'm glad you're on it and the direction it takes is ok for me.
Best
Okay, done something ... in addition to amd, cjs, esm I also added iife - that may well be the most commonly used option, and it is also the version now used in the karma tests. It's the global definition on window.
So I fiddled a bit with the build file to make it simpler, I added tests for the three browser loading formats (amd, esm, iife) and fixed some bugs.
You can run the browser tests by starting a server and accessing localhost:port/tests/. Please have a look at https://github.com/schrotie/gettext.js/blob/master/tests/amd.js My requirejs days are happily long over and I only half understand what I'm doing there - but having to shim gettext is certainly bad. Do you know what to do about this? I guess the filename would need to change to make requirejs happy ...
I removed this.__version = ... from the code. Shouldn't that maybe live on the i18n Class? Or is it required for amd or cjs?
Finally: What about the previous version? All tests may be fine, but this change would break existing client code. However, restoring the previous module-system-detection-version ... hm, could probably be done. You want that?
Cheers
Please have a look at
Hi @schrotie, nice work!
I'll try to use your fork on an old project using requirejs to observe the changes on the lib.
I removed this.__version = ... from the code. Shouldn't that maybe live on the i18n Class? Or is it required for amd or cjs?
We used that on our old days, for for a tiny lib as gettext I think a version into the code/object is not really mandatory IMO
Finally: What about the previous version?
I think all these changes could be considered as breaking changes, we'll have to do a major update (that way, It won't auto upgrade for users that use ^2.x in their package.json)
I'll have a bit more time in July, my pro and personal schedule is quite hectic right now --' Could you please ping me in 15-ish days from now to ensure I had time to look at it?
Thanks a lot
Best
Hi there,
Thanks for the ping :)
I managed to make the amd rollup generated file working by making the define statement anonymous. Eg:
Changing
define('i18n', function () { 'use strict';
[...]to
define(function () { 'use strict';
[...]It then works like a charm. I do remember that we had issues with named/not named required plugins, but not exactly why. Do you have by any chance a way to fix that in your rollup script?
Best
It works like a charm ;)
Maybe its time to go ahead and submit a PR? ;)
Best
This is merged and tagged 0.8.0! Thanks again for your great job 💯
It was truly a pleasure, working with you. Thanks for taking this up!