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

Add destructuring support to new-modules-import rule #144

Merged
merged 2 commits into from
Nov 16, 2017
Merged

Add destructuring support to new-modules-import rule #144

merged 2 commits into from
Nov 16, 2017

Conversation

clcuevas
Copy link
Contributor

This PR updates the new-modules-import rule to account for destructuring assignments. Right now when enabling this rule, it will not report on Ember properties without the Ember.. Below is an example:

import Ember from 'ember';

// This does not get reported to ESLint
const { Component } = Ember;

export default Component.extend({ ... });

However...

import Ember from 'ember';

// This does get reported to ESLint
export default Ember.Component.extend({ ... });

I'd like to take advantage of this awesome rule and turn it on for the Ember applications I'm currently working on. However, all of my applications use destructuring assignments and the current rule's state does not send/report any ESLint errors like I'd like it to.

@caseywatts
Copy link
Contributor

Thanks for working on this @clcuevas !
+1 do want :)

}

function populateMessage(obj) {
const isNamespace = EMBER_NAMESPACES.indexOf(`${obj.parent}.${obj.key}`) !== -1;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this could use a comment. what is this doing and why is it only relevant for the injections?

@Turbo87 Turbo87 changed the title Update new-modules-import Rule Add destructuring support to new-modules-import rule Nov 15, 2017
Copy link
Member

@rwjblue rwjblue left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks pretty good to me. Can you add an invalid test for:

import Ember from 'ember';

const { 
  computed: {
    alias,
    uniq
  }
} = Ember;

@clcuevas
Copy link
Contributor Author

@rwjblue @Turbo87 Your feedback has been addressed. Thanks!

Copy link
Member

@rwjblue rwjblue left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome, thank you!

@rwjblue rwjblue merged commit 1633f31 into ember-cli:master Nov 16, 2017
@clcuevas clcuevas deleted the newModules branch November 20, 2017 22:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants