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 babel-plugin for object spread syntax to babel-preset-jest #4519

Merged
merged 1 commit into from
Sep 21, 2017

Conversation

SimenB
Copy link
Member

@SimenB SimenB commented Sep 20, 2017

Summary
Seems like the easiest fix to #4248.

(typos in the commit message are the best)

Test plan
Running this version makes tests using object spread on node 8 pass without manually including the babel plugin there. Note that this doesn't transpile the syntax, it only allows babel to parse it.
Not sure if a test in this repo makes sense as it already gets that syntax plugin transitively in its own .babelrc

@SimenB SimenB changed the title Add abbel-plugin for object spread syntax to babel-preset-jest Add babel-plugin for object spread syntax to babel-preset-jest Sep 20, 2017
Copy link
Collaborator

@thymikee thymikee left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@cpojer cpojer merged commit d706f20 into jestjs:master Sep 21, 2017
@cpojer
Copy link
Member

cpojer commented Sep 21, 2017

Makes sense!

@digitalkaoz
Copy link

digitalkaoz commented Oct 6, 2017

@cpojer @SimenB object rest spread works fine during tests, but it fails again if i run jest with coverage

jest --coverage

@SimenB
Copy link
Member Author

SimenB commented Oct 6, 2017

@digitalkaoz that's probably because the istanbul instrumentation is outdated on your end. What does npm ls istanbul-lib-instrument say? You Need [email protected].

istanbuljs/istanbuljs#82

@digitalkaoz
Copy link

digitalkaoz commented Oct 6, 2017

@SimenB it seems i have 1.8.0

[root@d3e9f6900b3f app]# npm -g ls istanbul-lib-instrument
/usr/lib
└─┬ [email protected]
  └─┬ [email protected]
    ├─┬ [email protected]
    │ └── [email protected]  deduped
    ├── [email protected]
    └─┬ [email protected]
      └─┬ [email protected]
        └── [email protected]  deduped

@SimenB
Copy link
Member Author

SimenB commented Oct 6, 2017

Hmm, that's odd. What error are you getting?

@digitalkaoz
Copy link

/var/app/source.js: Unexpected token (29:37)
      > 29 |                 foo[name].options = {...globalConfig, ...foo[name].options};
           |                                      ^
        30 |             }
        31 |         });
        32 | 

@SimenB
Copy link
Member Author

SimenB commented Oct 6, 2017

I can't reproduce on my machine.

index.js

const obj = {
    foo: 'bar'
};

module.exports = {
    ...obj,
    foobar: 'baz'
};

test.js:

const file = require('./');

test('something', () => {
    expect(file).toEqual({
        foo: 'bar',
        foobar: 'baz'
    });
});
$ yarn jest --coverage
 PASS  ./test.js
  ✓ something (3ms)

Test Suites: 1 passed, 1 total
Tests:       1 passed, 1 total
Snapshots:   0 total
Time:        0.787s, estimated 1s
Ran all test suites.
----------|----------|----------|----------|----------|----------------|
File      |  % Stmts | % Branch |  % Funcs |  % Lines |Uncovered Lines |
----------|----------|----------|----------|----------|----------------|
All files |      100 |      100 |      100 |      100 |                |
 index.js |      100 |      100 |      100 |      100 |                |
----------|----------|----------|----------|----------|----------------|

On node 8.6.0

@digitalkaoz
Copy link

mh same setup here, gives me:

[root@d3e9f6900b3f tmp]# jest .
 PASS  ./test.js
  ✓ something (6ms)

Test Suites: 1 passed, 1 total
Tests:       1 passed, 1 total
Snapshots:   0 total
Time:        0.847s
Ran all test suites matching /./i.

[root@d3e9f6900b3f tmp]# jest --coverage
 FAIL  ./test.js
  ● Test suite failed to run

    /tmp/index.js: Unexpected token (6:4)
        4 |
        5 | module.exports = {
      > 6 |     ...obj,
          |     ^
        7 |     foobar: 'baz'
        8 | };
        9 |

Test Suites: 1 failed, 1 total
Tests:       0 total
Snapshots:   0 total
Time:        1.105s
Ran all test suites.
----------|----------|----------|----------|----------|----------------|
File      |  % Stmts | % Branch |  % Funcs |  % Lines |Uncovered Lines |
----------|----------|----------|----------|----------|----------------|
All files |  Unknown |  Unknown |  Unknown |  Unknown |                |
----------|----------|----------|----------|----------|----------------|
[root@d3e9f6900b3f tmp]# node --version
v8.6.0
[root@d3e9f6900b3f tmp]# npm --version
5.4.2
[root@d3e9f6900b3f tmp]# npm -g ls jest
/usr/lib
└── [email protected]

[root@d3e9f6900b3f tmp]# npm -g ls istanbul-lib-instrument
/usr/lib
└─┬ [email protected]
  └─┬ [email protected]
    ├─┬ [email protected]
    │ └── [email protected]  deduped
    ├── [email protected]
    └─┬ [email protected]
      └─┬ [email protected]
        └── [email protected]  deduped

@digitalkaoz
Copy link

digitalkaoz commented Oct 6, 2017

mh if i install jest locally, it works...

[root@d3e9f6900b3f tmp]# npm ls istanbul-lib-instrument
/tmp
└─┬ [email protected]
  └─┬ [email protected]
    ├─┬ [email protected]
    │ └── [email protected]  deduped
    ├── [email protected]
    └─┬ [email protected]
      └─┬ [email protected]
        └── [email protected]  deduped

@SimenB any idea on this?

@github-actions
Copy link

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 13, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants