Shopify / javascript

The home for all things JavaScript at Shopify.
MIT License
253 stars 38 forks source link

Decaf: investigate array comprehension issue #66

Closed lemonmade closed 8 years ago

lemonmade commented 8 years ago

In app/assets/javascripts/admin/modules/background_queue_poller.coffee, the following:

screen shot 2016-05-09 at 9 28 10 am

Gets transformed to this through esify:

screen shot 2016-05-02 at 4 30 47 pm

This code is weird and bad anyways, so maybe we can just kill it, but we should check to make sure that other comprehensions are not also getting removed like this.

GoodForOneFare commented 8 years ago

Confirmed that this is also a problem in the decaf mainline (it silently drops code in 0.4.3, and bails with an exception in 0.4.4+.

GoodForOneFare commented 8 years ago

Mainline decaf has a minor fix that works for a low percentage of our when statements. Manually unrolling all of these is risk prone, so I'm going to fix our decaf fork.

List of for loops that use when guards:

admin/lib/autocomplete/abstract_popover_autocomplete.coffee
admin/lib/autocomplete_v2/renderers/country_renderer.coffee
admin/lib/autocomplete_v2/renderers/render_tools.coffee
admin/lib/autocomplete_v2/renderers/shipping_zone_renderer.coffee
admin/lib/confirm_leave.coffee
admin/lib/error_tracking.coffee
admin/modules/background_queue_poller.coffee
admin/modules/bulk_operations.coffee
admin/modules/rte/rte_asset.coffee
admin/modules/sticky_table.coffee
admin/modules/taggable.coffee
admin/modules/template_editor.coffee
design_mode/location_behavior.coffee
design_mode_next/location_behavior.coffee
design_mode_next/utils/fast_dom.coffee
sello/components/ui_list.coffee
sello/utilities/binding.coffee
theme_editor/general_settings.coffee
theme_editor/util/state.coffee
theme_editor_next/block.coffee
theme_editor_next/blocks.coffee
theme_editor_next/dirty_tracker.coffee
theme_editor_next/general_settings.coffee
theme_editor_next/section.coffee
theme_editor_next/sections.coffee
theme_editor_next/sidebar/block_list.coffee
theme_editor_next/sidebar/block_picker.coffee
theme_editor_next/theme_editor.coffee
turbo/bar.coffee
GoodForOneFare commented 8 years ago

Not a problem after for/guard fixes. background-queue-poller.js is now decaffing to:

  jobMatchesParams(job) {
    var ref1;
    var ref;

    switch (this.matcher) {
    case 'all_of':
      for (var param of Object.keys(this.job_params)) {
        if ((((ref = job.params) != null ? ref[param] : void 0)) !== this.job_params[param]) {
          return false;
        }
      }

      break;
    case 'includes':
      for (var param of Object.keys(this.job_params)) {
        if (_.intersection(this.job_params[param], (ref1 = job.params) != null ? ref1[param] : void 0).length === 0) {
          return false;
        }
      }
    }

    return true;
  }