RefugeRestrooms / refugerestrooms

REFUGE restrooms indexes and maps safe restroom locations for trans, intersex, and gender nonconforming individuals.
http://www.refugerestrooms.org
GNU Affero General Public License v3.0
894 stars 261 forks source link

Decaffeinate #474

Closed stardust66 closed 6 years ago

stardust66 commented 6 years ago

Context

Summary of Changes

Two problems with this approach. First, the files that are in es6 must have the extension .es6 to be converted. Second, the converted files are automatically in strict mode. I couldn't figure out how to configure Babel when using sprockets-es6, since it uses babel through babel-source and babel-transpiler. .babelrc doesn't work, see https://github.com/babel/ruby-babel-transpiler/issues/287. No configuration options are documented for sprockets-es6 and there doesn't seem like there are any based on the code. Also, sprockets-es6 is a temporary solution as Sprockets 4 natively supports es6. However, it's still in beta right now.

I can try to use webpacker as a solution but that might involve bigger changes. Although, the best solution is to switch to Sprockets 4 when a stable version gets released. What do you guys think?

Checklist

mi-wood commented 6 years ago

@stardust66 is there any downside to using strict mode? I'm a little confused as to why generated code uses this, since the generation itself could potentially trigger exceptions in this case. I'm not really familiar though.

stardust66 commented 6 years ago

Strict mode: https://docs.microsoft.com/en-us/scripting/javascript/advanced/strict-mode-javascript

Strict mode throws errors when plain javascript doesn't. It restricts code for better errors and debugging. For example, in javascript you can implicitly declare variables and properties.

let test = {};
test.prop = "value";

This doesn't throw an error without strict mode. Babel uses strict mode by default because es6 modules use strict mode implicitly. I guess it's acceptable because it makes you write more proper code. If there's a way to configure sprockets-es6 I could probably turn it off but there doesn't seem to be.

DeeDeeG commented 6 years ago

I am not a JavaScript coder, pretty much, so take all this with a grain of salt.

But as I see it, it seems like the choice to merge this is around whether JS coders for our project will prefer CoffeeScript or standard JavaScript going forward.

I see strict mode as being not much of a big deal, since you can still do what you need to do, it just enforces more explicit/"correct" coding... (according to strict mode's notions of "correctness.")

So if people are really hungering for standard JS code in the repo, merging this would accomplish that goal. It might not theoretically be the absolute perfect way to do it, (I wouldn't know either way) but it is a way, so it seems like it could be a good thing.

(And not that this matters a ton, but I personally find standard JS a bit more readable than CoffeeScript.)

(I found another strict mode reference, in case that's useful: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Strict_mode )

DeeDeeG commented 6 years ago

There is also this simpler coffescript to js converter: http://js2.coffee/ (aka https://github.com/js2coffee/js2coffee)

Not 100% sure if that is producing non es6 code (I can't tell es6 from non-es6 JavaScript), but I think so.