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.

Sign up for GitHub

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

Jump to bottom

[import/named] fix destructuring assignemnt #1232

Merged
merged 2 commits into from
Nov 16, 2018
Merged

[import/named] fix destructuring assignemnt #1232

merged 2 commits into from
Nov 16, 2018

Conversation

ljqx
Copy link
Contributor

@ljqx ljqx commented Nov 15, 2018

This PR fixes #718.

The fix is simple, just recognize the AssignmentPattern and put the name into the exports map.

@coveralls
Copy link

Coverage Status

Coverage decreased (-3.7%) to 93.617% when pulling 64baa91 on ljqx:import-named-assignment into db471a8 on benmosher:master.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage decreased (-3.7%) to 93.617% when pulling 64baa91 on ljqx:import-named-assignment into db471a8 on benmosher:master.

@coveralls
Copy link

coveralls commented Nov 15, 2018 •
edited
Loading

Coverage Status

Coverage increased (+0.003%) to 97.36% when pulling 9a13f81 on ljqx:import-named-assignment into db471a8 on benmosher:master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-3.7%) to 93.617% when pulling 64baa91 on ljqx:import-named-assignment into db471a8 on benmosher:master.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage decreased (-3.7%) to 93.617% when pulling 64baa91 on ljqx:import-named-assignment into db471a8 on benmosher:master.

ljharb
ljharb approved these changes Nov 15, 2018
View reviewed changes
Copy link
Member

@ljharb ljharb left a comment

Choose a reason for hiding this comment

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

Looks great!

@ljharb ljharb added the bug label Nov 15, 2018
@ljharb
Copy link
Member

ljharb commented Nov 15, 2018

(looks like tests are failing; you’ll need to update an 8 to a 10 somewhere)

@ljqx
Copy link
Contributor Author

ljqx commented Nov 16, 2018

@ljharb , thanks! fixed.

Actually on my local machine (Windows 10 and Ubuntu on Windows 10) I've many test failures even no change is made on master for npm test. Is this known issue? Or it's related to configuration on my local machine?

The error log:

  1411 passing (8s)
  27 failing

  1) ExportMap deprecation metadata default parser deprecated imports works with named imports.:
     AssertionError: expected {} to have a deep property 'doc.tags[0].title'
      at Context.<anonymous> (D:/src/eslint-plugin-import/tests/src/core/getExports.js:113:27)

  2) ExportMap deprecation metadata default parser deprecated imports works with default imports.:
     AssertionError: expected {} to have a deep property 'doc.tags[0].title'
      at Context.<anonymous> (D:/src/eslint-plugin-import/tests/src/core/getExports.js:122:43)

  3) ExportMap deprecation metadata default parser deprecated imports works with variables.:
     AssertionError: expected {} to have a deep property 'doc.tags[0].title'
      at Context.<anonymous> (D:/src/eslint-plugin-import/tests/src/core/getExports.js:130:43)

  4) ExportMap deprecation metadata default parser deprecated imports multi-line variables works for the first one:
     AssertionError: expected {} to have a deep property 'doc.tags[0].title'
      at Context.<anonymous> (D:/src/eslint-plugin-import/tests/src/core/getExports.js:141:45)

  5) ExportMap deprecation metadata default parser deprecated imports multi-line variables works for the second one:
     AssertionError: expected {} to have a deep property 'doc.tags[0].title'
      at Context.<anonymous> (D:/src/eslint-plugin-import/tests/src/core/getExports.js:150:45)

  6) ExportMap deprecation metadata default parser deprecated imports multi-line variables works for the third one, etc.:
     AssertionError: expected {} to have a deep property 'doc.tags[0].title'
      at Context.<anonymous> (D:/src/eslint-plugin-import/tests/src/core/getExports.js:159:45)

  7) ExportMap deprecation metadata babel-eslint deprecated imports works with named imports.:
     AssertionError: expected {} to have a deep property 'doc.tags[0].title'
      at Context.<anonymous> (D:/src/eslint-plugin-import/tests/src/core/getExports.js:113:27)

  8) ExportMap deprecation metadata babel-eslint deprecated imports works with default imports.:
     AssertionError: expected {} to have a deep property 'doc.tags[0].title'
      at Context.<anonymous> (D:/src/eslint-plugin-import/tests/src/core/getExports.js:122:43)

  9) ExportMap deprecation metadata babel-eslint deprecated imports works with variables.:
     AssertionError: expected {} to have a deep property 'doc.tags[0].title'
      at Context.<anonymous> (D:/src/eslint-plugin-import/tests/src/core/getExports.js:130:43)

  10) ExportMap deprecation metadata babel-eslint deprecated imports multi-line variables works for the first one:
     AssertionError: expected {} to have a deep property 'doc.tags[0].title'
      at Context.<anonymous> (D:/src/eslint-plugin-import/tests/src/core/getExports.js:141:45)

  11) ExportMap deprecation metadata babel-eslint deprecated imports multi-line variables works for the second one:
     AssertionError: expected {} to have a deep property 'doc.tags[0].title'
      at Context.<anonymous> (D:/src/eslint-plugin-import/tests/src/core/getExports.js:150:45)

  12) ExportMap deprecation metadata babel-eslint deprecated imports multi-line variables works for the third one, etc.:
     AssertionError: expected {} to have a deep property 'doc.tags[0].title'
      at Context.<anonymous> (D:/src/eslint-plugin-import/tests/src/core/getExports.js:159:45)

  13) no-deprecated invalid import { fn } from './deprecated':

      AssertionError [ERR_ASSERTION]: Should have 1 error but had 0: []
      + expected - actual

      -0
      +1

      at testInvalidTemplate (node_modules\eslint\lib\testers\rule-tester.js:462:24)
      at Context.RuleTester.it (node_modules\eslint\lib\testers\rule-tester.js:592:25)

  14) no-deprecated invalid import TerribleClass from './deprecated':

      AssertionError [ERR_ASSERTION]: Should have 1 error but had 0: []
      + expected - actual

      -0
      +1

      at testInvalidTemplate (node_modules\eslint\lib\testers\rule-tester.js:462:24)
      at Context.RuleTester.it (node_modules\eslint\lib\testers\rule-tester.js:592:25)

  15) no-deprecated invalid import { MY_TERRIBLE_ACTION } from './deprecated':

      AssertionError [ERR_ASSERTION]: Should have 1 error but had 0: []
      + expected - actual

      -0
      +1

      at testInvalidTemplate (node_modules\eslint\lib\testers\rule-tester.js:462:24)
      at Context.RuleTester.it (node_modules\eslint\lib\testers\rule-tester.js:592:25)

  16) no-deprecated invalid import { fn } from './deprecated':

      AssertionError [ERR_ASSERTION]: Should have 1 error but had 0: []
      + expected - actual

      -0
      +1

      at testInvalidTemplate (node_modules\eslint\lib\testers\rule-tester.js:462:24)
      at Context.RuleTester.it (node_modules\eslint\lib\testers\rule-tester.js:592:25)

  17) no-deprecated invalid import { MY_TERRIBLE_ACTION } from './deprecated'; function shadow(MY_TERRIBLE_ACTION) { console.log(MY_TERRIBLE_ACTION); }:

      AssertionError [ERR_ASSERTION]: Should have 1 error but had 0: []
      + expected - actual

      -0
      +1

      at testInvalidTemplate (node_modules\eslint\lib\testers\rule-tester.js:462:24)
      at Context.RuleTester.it (node_modules\eslint\lib\testers\rule-tester.js:592:25)

  18) no-deprecated invalid import { MY_TERRIBLE_ACTION, fine } from './deprecated'; console.log(fine):

      AssertionError [ERR_ASSERTION]: Should have 1 error but had 0: []
      + expected - actual

      -0
      +1

      at testInvalidTemplate (node_modules\eslint\lib\testers\rule-tester.js:462:24)
      at Context.RuleTester.it (node_modules\eslint\lib\testers\rule-tester.js:592:25)

  19) no-deprecated invalid import { MY_TERRIBLE_ACTION } from './deprecated'; console.log(MY_TERRIBLE_ACTION):

      AssertionError [ERR_ASSERTION]: Should have 2 errors but had 0: []
      + expected - actual

      -0
      +2

      at testInvalidTemplate (node_modules\eslint\lib\testers\rule-tester.js:462:24)
      at Context.RuleTester.it (node_modules\eslint\lib\testers\rule-tester.js:592:25)

  20) no-deprecated invalid import { MY_TERRIBLE_ACTION } from './deprecated'; console.log(someOther.MY_TERRIBLE_ACTION):

      AssertionError [ERR_ASSERTION]: Should have 1 error but had 0: []
      + expected - actual

      -0
      +1

      at testInvalidTemplate (node_modules\eslint\lib\testers\rule-tester.js:462:24)
      at Context.RuleTester.it (node_modules\eslint\lib\testers\rule-tester.js:592:25)

  21) no-deprecated invalid import { MY_TERRIBLE_ACTION } from './deprecated'; console.log(MY_TERRIBLE_ACTION.whatever()):

      AssertionError [ERR_ASSERTION]: Should have 2 errors but had 0: []
      + expected - actual

      -0
      +2

      at testInvalidTemplate (node_modules\eslint\lib\testers\rule-tester.js:462:24)
      at Context.RuleTester.it (node_modules\eslint\lib\testers\rule-tester.js:592:25)

  22) no-deprecated invalid import { MY_TERRIBLE_ACTION } from './deprecated'; console.log(MY_TERRIBLE_ACTION(this, is, the, worst)):

      AssertionError [ERR_ASSERTION]: Should have 2 errors but had 0: []
      + expected - actual

      -0
      +2

      at testInvalidTemplate (node_modules\eslint\lib\testers\rule-tester.js:462:24)
      at Context.RuleTester.it (node_modules\eslint\lib\testers\rule-tester.js:592:25)

  23) no-deprecated invalid import * as depd from './deprecated'; console.log(depd.MY_TERRIBLE_ACTION):

      AssertionError [ERR_ASSERTION]: Should have 1 error but had 0: []
      + expected - actual

      -0
      +1

      at testInvalidTemplate (node_modules\eslint\lib\testers\rule-tester.js:462:24)
      at Context.RuleTester.it (node_modules\eslint\lib\testers\rule-tester.js:592:25)

  24) no-deprecated invalid import * as deep from './deep-deprecated'; console.log(deep.deepDep.MY_TERRIBLE_ACTION):

      AssertionError [ERR_ASSERTION]: Should have 1 error but had 0: []
      + expected - actual

      -0
      +1

      at testInvalidTemplate (node_modules\eslint\lib\testers\rule-tester.js:462:24)
      at Context.RuleTester.it (node_modules\eslint\lib\testers\rule-tester.js:592:25)

  25) no-deprecated invalid import { deepDep } from './deep-deprecated'; console.log(deepDep.MY_TERRIBLE_ACTION):

      AssertionError [ERR_ASSERTION]: Should have 1 error but had 0: []
      + expected - actual

      -0
      +1

      at testInvalidTemplate (node_modules\eslint\lib\testers\rule-tester.js:462:24)
      at Context.RuleTester.it (node_modules\eslint\lib\testers\rule-tester.js:592:25)

  26) no-deprecated invalid import { deepDep } from './deep-deprecated'; function x(deepNDep) { console.log(deepDep.MY_TERRIBLE_ACTION) }:

      AssertionError [ERR_ASSERTION]: Should have 1 error but had 0: []
      + expected - actual

      -0
      +1

      at testInvalidTemplate (node_modules\eslint\lib\testers\rule-tester.js:462:24)
      at Context.RuleTester.it (node_modules\eslint\lib\testers\rule-tester.js:592:25)

  27) no-deprecated: hoisting invalid console.log(MY_TERRIBLE_ACTION); import { MY_TERRIBLE_ACTION } from './deprecated':

      AssertionError [ERR_ASSERTION]: Should have 2 errors but had 0: []
      + expected - actual

      -0
      +2

      at testInvalidTemplate (node_modules\eslint\lib\testers\rule-tester.js:462:24)
      at Context.RuleTester.it (node_modules\eslint\lib\testers\rule-tester.js:592:25)

@ljharb
Copy link
Member

ljharb commented Nov 16, 2018

Since our appveyor tests are passing on windows, I’d expect it to be passing.

Maybe you could file a separate issue about that?

@ljharb ljharb merged commit 798eed7 into import-js:master Nov 16, 2018
@ljqx
Copy link
Contributor Author

ljqx commented Nov 16, 2018

Thanks @ljharb , I've found the cause and sent out an issue for that, see #1233.

This was referenced Sep 16, 2021
Open
Open
This was referenced Dec 31, 2021
Open
Open
Open
This was referenced Jun 21, 2023
Open
Open
Open
This was referenced Nov 30, 2023
Open
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bigslycat bigslycat bigslycat approved these changes

@ljharb ljharb ljharb approved these changes

Assignees
No one assigned
Labels
bug
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

import/named works fail with destructuring with default values
4 participants
@ljqx @coveralls @ljharb @bigslycat