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

Convert operations(operator, motion, text-object) to JS #895

Merged
merged 100 commits into from
Oct 6, 2017

Conversation

t9md
Copy link
Owner

@t9md t9md commented Sep 25, 2017

To let me understand how it is, clarify requirement.

TODO( 🚢 released)

Migration best practices memo

  • Basic rule is to move logic to from constructor or initialize to execute().
  • In ES6 class's constructor, super must be called first to refer this.
  • So we no longer do following logic.
    • Old way in CoffeeScript
      1. set this.prop
      2. then call super
      3. In parent's constructor behave differently based on this.prop
    • New way in ES6
      1. always call super first in constructor
      2. set this.prop
      3. in execute, behave differently based on this.prop

Working memo

before conversion

had 2 layer

  • klass.prototype.ivar
  • instance.ivar

after conversion

only 1 layer (no prototype.ivar layer)

  • instance.ivar

Caution

  • ES6 class is not extendable by CS's class.
  • Thus, converting breaks user's custom class in their init.coffee(if exists) or vmp-plugin package written in coffee.
  • CS's executable class body is no longer availale.
  • Class prop is not set on prototype, and it's set after constructor's super call.
  • So don't expect these class props are available in parent's constructor.
    • Basically keep constructor function small and simple.
  • In CS, Object.assign(this, prop) in constructor can overwrite properties defined in prototype, but in JS, properties are set after construction, so this approach must be replaced with new one.
    • I experimentally introduce Base.assign as replacement for constructor's 2nd arg(was prop directly Object.assign-ed in constructor.

@t9md t9md changed the title [Tryout] Convert misc-commands.coffee to JS [Tryout] Convert operations code from CoffeeScript to JS Sep 26, 2017
@t9md t9md changed the title [Tryout] Convert operations code from CoffeeScript to JS Convert operations code from CoffeeScript to JS Oct 5, 2017
@t9md t9md changed the title Convert operations code from CoffeeScript to JS Convert all operations(operator, motion, text-object) from CoffeeScript to JS Oct 5, 2017
@t9md t9md changed the title Convert all operations(operator, motion, text-object) from CoffeeScript to JS Convert operations(operator, motion, text-object) to JS Oct 5, 2017
@t9md t9md closed this Oct 5, 2017
@t9md t9md deleted the convert-misc-to-js branch October 5, 2017 04:30
@t9md t9md restored the convert-misc-to-js branch October 5, 2017 04:31
@t9md t9md reopened this Oct 5, 2017
@t9md
Copy link
Owner Author

t9md commented Oct 6, 2017

Will merge. I won't go back anyway.
I like JS now over CoffeeScript basically because of prittier.

@t9md t9md merged commit 0bb2ab8 into master Oct 6, 2017
@t9md t9md deleted the convert-misc-to-js branch November 12, 2017 09:34
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

Successfully merging this pull request may close these issues.

1 participant