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

Version 1.5.3 'leaks' ES6 code (let, const) into minified js bundle when built #38

Closed
spencern opened this issue Apr 11, 2017 · 10 comments

Comments

@spencern
Copy link

spencern commented Apr 11, 2017

I've got an issue that seems related to #28

I've got a meteor app (meteor controls the build) that imports transliteration - there's a description of the problem and a reproduction based on our app here: reactioncommerce/reaction#2091

For versions up to 1.5.2 transliteration bundles a correctly babelified and minified js file. When we got bumped up to 1.5.3, we started seeing let and const in our minified production bundle. I think this is due to the change for 1.5.3 that removed the "browser": "lib/browser/transliteration.js", line, and the conversation in #28 seems to point that direction as well.

This issue only causes problems for us when an older browser such as PhantomJS is used to crawl the site. Unfortunately, the prerendering techniques we use to create cached, google/bing/yahoo crawl friendly pages use PhantomJS, and the let and const in the js bundle causes them to crash before rendering.

@dzcpy
Copy link
Owner

dzcpy commented Apr 11, 2017

Hi,
Thanks for the spot, I'll investigate this issue today and get back to you a bit later. Adding "browser": "lib/browser/transliteration.js" actually causes other problems, someone had reported an issue running it in reactnative. It's a bit hard for me to make it compatible to all platforms. By the way, have you tried to manually import it from lib/browser/transliteration?
So it could be something like this:
var transliteration = require('lib/browser/transliteration')

@lcampanis
Copy link

Hi @spencern! Have a look at #39

@spencern
Copy link
Author

@andyhu
I've not tried manually importing it yet. I can give that a shot. Thanks for the prompt response

@spencern
Copy link
Author

@lcampanis I'll take a look!

dzcpy pushed a commit that referenced this issue May 15, 2017
@dzcpy
Copy link
Owner

dzcpy commented May 15, 2017

Hi, sorry for the lack of response, I've been very busy recently. I have done some work to fix this issue. Could you test again with version 1.6.2? If there's still any issues, please let me know. Thanks!

@spencern
Copy link
Author

I'll do that @andyhu. Thanks

@kachkaev
Copy link

See #44 - might help

@spencern
Copy link
Author

It appears that this is working for us now with the latest version of transliteration (1.6.2) by importing the minified version.
e.g.

import "transliteration/lib/browser/transliteration.js";

Closing this issue for now

@kachkaev
Copy link

kachkaev commented Jun 26, 2017

It is discouraged to use pre-built packages in webpack, because this way you import all the code without giving webpack an opportunity to only bundle the stuff that's actually being used.

Please do not use import "transliteration/lib/browser/transliteration.js"; in production!

@spencern
Copy link
Author

Hi @kachkaev - I agree that it's not ideal, but it was suggested to us by @andyhu and is currently working. We're using Meteor and not webpack currently and working on moving transliteration to a dynamic import as well so it's only loaded for users who need it.

I saw your PR, if that gets pulled in, we'll try that as well and see if we can stop importing the pre-built package.

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

No branches or pull requests

4 participants