Skip to content
This repository was archived by the owner on Aug 8, 2023. It is now read-only.

Make IIFE wrap inside to avoid conflict with no-extra-parens#62

Merged
marlowpayne merged 1 commit intomasterfrom
wrap-iife-conflict
Aug 5, 2015
Merged

Make IIFE wrap inside to avoid conflict with no-extra-parens#62
marlowpayne merged 1 commit intomasterfrom
wrap-iife-conflict

Conversation

@marlowpayne
Copy link
Copy Markdown

Status: Ready for Review
Reviewers: @donnielrt @Helen-Mobify

Changes

  • Our current wrap-iife rule defaults to "outside" wrapping required
// wrapping the call expression (outside)
return (function () { return { y: 1 };}());

However, this will conflict with our rule for no-extra-parens, and since the rule is set to "outside" we cannot avoid a linting error since wrapping via inside to avoid no-extra-parens i.e.

// wrapping the function expression (inside)
return (function () { return { y: 1 };})();

will violate the default "outside" rule.
This PR changes our rule to "inside" wrapping so that we may reach a no lint error state.

TL;DR: With the "outside" IIFE wrapping rule, you'll always be in an error state. Changing the IIFE rule to "inside" will let us reach a conflict-free state with no lint errors.

Todos:

  • Engineer +1

Links:

Eslint wrap-iife rule

How to Test

  • Verify that we enter a constant lint error state for IIFEs
  • Update your local copy of the code-style by running npm install -g git+ssh://git@github.com:mobify/mobify-code-style.git#wrap-iife-conflict
  • Verify that we may now wrap IIFEs via "inside" parens, to avoid linting errors

@donnielrt
Copy link
Copy Markdown
Contributor

@marlowpayne I'm a little confused, we don't seem to be getting a lint error for the outside wrapping of the IIFE:

screenshot 2015-07-16 09 54 39

Am I misunderstanding what you're saying?

@marlowpayne
Copy link
Copy Markdown
Author

Ah sorry @donnielrt I grabbed those code samples from the Eslint site, not realizing that my cause for the PR is a bit more specific. Try the IIFE in a return statement.

// wrapping the call expression (outside)
return (function() { return { y: 1 };}());

@marlowpayne
Copy link
Copy Markdown
Author

It's been a while since anyone's looked at this. Mind taking a quick look, @Helen-Mobify ?

@wizardlyhel
Copy link
Copy Markdown

@marlowpayne

I do see the error for the code example you have.
screen shot 2015-08-05 at 11 41 48 am

screen shot 2015-08-05 at 11 41 38 am

However, even with this updated mobify-code-style branch, I don't see error disappear.

@marlowpayne
Copy link
Copy Markdown
Author

@Helen-Mobify thanks for taking a look. On this branch you should be able to reach a non-error state for "inside" wrapping IIFEs. Are you able to get a non-error state with this?:

test: function() {
    return (function () { return { y: 1 };})();
}

@wizardlyhel
Copy link
Copy Markdown

@marlowpayne The screenshot I included are the errors I got for that piece of code.

@wizardlyhel
Copy link
Copy Markdown

👍 Tested!

The working code is to have the wrapping inside.

return (function() { return { y: 1 };})();

marlowpayne pushed a commit that referenced this pull request Aug 5, 2015
Make IIFE wrap inside to avoid conflict with no-extra-parens
@marlowpayne marlowpayne merged commit 91db4f9 into master Aug 5, 2015
@marlowpayne marlowpayne deleted the wrap-iife-conflict branch August 5, 2015 22:42
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants