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

const in output JS? #272

Closed
sgehrman opened this issue May 31, 2018 · 24 comments
Closed

const in output JS? #272

sgehrman opened this issue May 31, 2018 · 24 comments

Comments

@sgehrman
Copy link

sgehrman commented May 31, 2018

Hey, I'm not much of a JS guy, but I've had complaints from people on old versions of Safari.

Older versions don't like const keyword.

Using webpack and const is in my build. Let me know if I did something wrong.

*!

  • VERSION: 2.0.1
  • DATE: 2018-05-30
  • UPDATES AND DOCS AT: http://greensock.com
  • @license Copyright (c) 2008-2018, GreenSock. All rights reserved.
  • This work is subject to the terms at http://greensock.com/standard-license or for
  • Club GreenSock members, the software agreement that was issued with your membership.
  • @author: Jack Doyle, jack@greensock.com
    */
    const r="undefined"!=typeof window?window:void 0!==t&&t.exports&&void 0!==i?i:{},o=function(t,e){var n={},i=t.document,r=t.GreenSockGlobals=t.GreenSockGlobals||t;if(r.TweenLite)return r.TweenLite;var o,a,s,l,c,u,p,d=function(t){var e,n=t.split("."),i=r;for(e=0;e<n.length;e++)i[n[e]]=i=i[n[e]]||{};return i},h=d("com.greensock"),f=function(t){var e,n=[],i=t.length;for(e=0;e!==i;n.push(t[e++]));return n},m=function(){},g=(u=Object.prototype.toString,p=u.call([]),function(t){return null!=t&&(t instanceof Array||"object"==typeof t&&!!t.push&&u.call(t)===p)}),b={},x=function(t,e,i,o){this.sc=b[t]?b[t].sc:[],b[t]=this,this.gsClass=null,this.func=i;var a=[];this.check=function(s){for(var l,c,u,p,h=e.length,f=h;--h>-1;)(l=b[e[h]]||new
@sgehrman
Copy link
Author

maybe I need the umd. I'll try that.

@sgehrman
Copy link
Author

import {
TweenMax,
Power2
} from 'gsap/umd'

Can't resolve 'gsap/umd'

webpack seems to think this is a relative path.

I could add node_modules/gsap to webpack so it transpiles that. But maybe you should fix the gsap package?

@sgehrman
Copy link
Author

sgehrman commented May 31, 2018

So, this seems to be a solution, but let me know if you have a better idea.

in webpack config.

{
      test: /\.js$/,
      loader: 'babel-loader',
      // exclude: /node_modules/,
      exclude: function(modulePath) {
        return /node_modules/.test(modulePath) &&
          !/node_modules\/gsap/.test(modulePath);
      }
    }

That will transpile only node_modules/gsap and not the rest of node_modules

gsap is the only package I used that includes const keyword which breaks old versions of Safari.

@jackdoyle
Copy link
Member

@sgehrman I think you just forgot to define the class you were importing from, like:

import { TweenMax, Power2 } from 'gsap/umd/TweenMax';

As for the old Safari thing, that's interesting; I didn't realize Webpack would leave ES6 stuff in there like that. Hm. Yeah, it'd be pretty easy to change those to "var" instead I guess. I can do that in the next release. But again, the UMD version should be work great for you - just define the class you're importing from.

@MasterJames
Copy link

To embrace 'let' as much as possible, is my recommendation. It's the proper way to var, so var is just a legacy mistake, while const is equally redundant and less flexible for hot-code loading being pushed out etc. Anyway it's not absolute but what I feel instinctively is best most of the time.

@jackdoyle
Copy link
Member

@MasterJames if one of the complaints is that old versions of Safari are choking on "const", I can't imagine "let" would be any different since it's also ES6. Plus I've read that let and const are both slower than "var" in some browsers. So my hunch is that it'd be best here to just switch it to "var". If anyone has objections, let me know.

@MasterJames
Copy link

The performance issue was a temporary issue that is no longer a concern. There is evidence of that out there. Maybe try in your personal situation and see if it makes a significant difference for you and your code on the target platform browser etc. [ P.S. 2 less chars is let or var to const, times all declarations.]
Anyway performance should not be the final word.
This thread I quickly dug up https://softwareengineering.stackexchange.com/questions/274342/is-there-any-reason-to-use-the-var-keyword-in-es6
which has a reference to Doug Crockford and his explanations [good videos on his personal sight I think] that is of a similar origin to my opinions on the subject. Rarely I diverge from his wisdom like don't bother with const [main reason is hot code loading I just overwrite code stored in object trees]. There's more to research on the subject just avoid anything too old on the subject as V8 optimizations are still ongoing so eventually they should be bested for the proper way to 'let'. It's like Doug says we would have fixed var but it was breaking in some poor legacy code so they were forced to keep the old badly done var and renamed the proper good corrected version 'let'.

@jackdoyle
Copy link
Member

@MasterJames Are you suggesting that using let instead of const (or var) will solve the Safari issue mentioned above? Do you think the old version of Safari will recognize let but not const?

I didn't want this thread to become about theoretical benefits of let/const over var - I'm more focused on ensuring compatibility at this point since that's such a keystone in GSAP. I do appreciate the link and suggestions though.

@MasterJames
Copy link

I bow to your perdicament. I am off topic then agreed.
Still I encourage you to pop up an upgrade warning saying upgrade or die trying. Don't encourage legacy fools to perpetuate their foolish fodder.
Devs that want or need to can dynamically load an old version.

@jackdoyle
Copy link
Member

I hear you, but I really don't want to force people to downgrade to an older version and miss out on feature enhancements and bug fixes just because of a let/const preference that's largely stylistic and totally inconsequential functionally (at least in this context).

If I misunderstood and you're actually trying to say that you think using "var" here will break things for people (thus it's imperative that we use let/const), please let me know.

@MasterJames
Copy link

MasterJames commented Jun 2, 2018

Checking Usage of said browser
Safari is only 3.3 percent
https://www.w3schools.com/BROWSERS/default.asp
and of that? oh look 0.0%
https://www.w3schools.com/BROWSERS/browsers_safari.asp

Right okay to be clear, no what I've been saying is move forward.
Only use 'let' don't bother with 'const'. Warn people their browsers are out of date and tell developers that request compatibilityand support for 0.0% usage browsers to dynamically load an old version of any/all libraries (in this case greensock). It won't break anything but it's not the right thing to keep using var, and 'let' is the right choice, while 'const' is more of a conditional thing to say it's not worth using. If you have really moved forward and build code that can be hot-loaded etc.

@azopyros
Copy link

Break from 1.20.4 to 1.20.6. I need to gsap: 1.20.4 because latter version put const, then minify do const{ then IE break.

@jackdoyle
Copy link
Member

@azopyros 1.20.6 doesn't have const at all, unless you're using it as a module because the package.json "module" does point to the /esm/ directory. You should be able to either avoid using it as a module or import directly from the root or even the /src/uncompressed/ directory if you prefer. Or use 1.20.4 of course.

@glebmachine
Copy link

glebmachine commented Jul 20, 2018

Yeah, that makes me cry, again.

"module" pointer should point to ES5 module, which only use import/export. No arrow functions / const / let / spread and other feature is allowed.

Further reading: https://github.com/rollup/rollup/wiki/pkg.module#wait-it-just-means-import-and-export--not-other-future-javascript-features

Please, replace const with var and then build will respects specs.

@glebmachine
Copy link

@azopyros @MasterJames @sgehrman guys, i've forked repo to https://github.com/glebmachine/GreenSock-JS and replaced const with var.

Everything work like a charm. Made fork for yourself, or use mine. Just add to package.json:

    "gsap": "https://github.com/glebmachine/GreenSock-JS.git",

@MasterJames
Copy link

Nice. Thanks.

@mattcolman
Copy link

can we please change const to var. it's breaking our iOS 9 users on safari.

@glebmachine
Copy link

@jackdoyle could you please?

@jackdoyle
Copy link
Member

Absolutely. 2.0.2 should be released sometime next week. Thanks for your patience :)

@MasterJames
Copy link

Wasn't 'let' the correct choice now. Maybe backfill it if no let found by setting let to var if it doesn't exist on some backwards incompatibility issue that should really just be forced to upgrade.

@jackdoyle
Copy link
Member

I switched to "var" to avoid backward compatibility issues. It's released now in 2.0.2. "let" is part of ES6 just like "const" is, so that wouldn't be any better.

@MasterJames
Copy link

MasterJames commented Aug 26, 2018

Okay cool.
Not sure if this could be made to work somehow anyway.
try {let tester = 'working';}
catch () {let = var}
You get the idea anyway.
Agh!... let them use babel if they must and there is no way to (monkey patch to) add a let that is a pointer to the old var.

@glebmachine
Copy link

@MasterJames read this spec first
https://github.com/rollup/rollup/wiki/pkg.module#wait-it-just-means-import-and-export--not-other-future-javascript-features

@MasterJames
Copy link

Thanks for the link.

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

6 participants