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

TypeError: Cannot set property logger of #<Object> which has only a getter #8363

Closed
jmikulka opened this issue Jul 23, 2018 · 6 comments
Closed
Labels
outdated A closed issue/PR that is archived due to age. Recommended to make a new issue

Comments

@jmikulka
Copy link

v7 Regression

First check out: https://new.babeljs.io/docs/en/next/v7-migration.html
Also a partial upgrade tool: https://github.com/babel/babel-upgrade

Potential Commit/PR that introduced the regression
N/A

Describe the regression
Running jest tests with the babel-preset-env@6 pass:

yarn run v1.7.0
warning package.json: No license field
$ jest
 PASS  ./deps.test.js
  Test
     logger has been called (5ms)

  console.log deps.test.js:4
    { logger: [Function] }

Test Suites: 1 passed, 1 total
Tests:       1 passed, 1 total
Snapshots:   0 total
Time:        1.277s
Ran all test suites.
  Done in 2.33s.

After upgrade to @babel/preset-env@7, the tests fail:

yarn run v1.7.0
warning package.json: No license field
$ jest
 FAIL  ./deps.test.js
  Test
     logger has been called (10ms)

   Test  logger has been called

    TypeError: Cannot set property logger of #<Object> which has only a getter

       6 |   const mockLogger = jest.fn();
       7 |   beforeEach(() => {
    >  8 |     jest.spyOn(deps, 'logger').mockImplementation(mockLogger);
         |          ^
       9 |   });
      10 |
      11 |   it('logger has been called', () => {

      at ModuleMockerClass.spyOn (node_modules/jest-mock/build/index.js:706:26)
      at Object.<anonymous> (deps.test.js:8:10)

   Test  logger has been called

    expect(jest.fn()).toHaveBeenCalledWith(expected)

    Expected mock function to have been called with:
      ["test"]
    But it was not called.

      11 |   it('logger has been called', () => {
      12 |     deps.logger('test');
    > 13 |     expect(mockLogger).toHaveBeenCalledWith('test');
         |                        ^
      14 |   });
      15 | });
      16 |

      at Object.<anonymous> (deps.test.js:13:24)

  console.log deps.test.js:4
    { logger: [Getter] }

  console.log logger.js:1
    LOGGER [ 'test' ]

Test Suites: 1 failed, 1 total
Tests:       1 failed, 1 total
Snapshots:   0 total
Time:        1.119s
Ran all test suites.
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.

Input Code
Clone https://github.com/jmikulka/babel_preset_env_regression and run:

BABEL_ENV=old yarn test # test passes, uses babel-preset-env

BABEL_ENV=new yarn test # test fails, uses @babel/preset-env

Babel Configuration (.babelrc, package.json, cli command)

// .babelrc.js
module.exports = {
  presets: [process.env.BABEL_ENV === 'old' ? 'env' : '@babel/preset-env'],
};

Expected behavior/code
The tests with the new preset should not fail.

Environment

  • Babel version(s): [v7.0.0-beta.54]
  • Node/npm version: [Node 8.11.3/npm 5.6.0]
  • OS: [OSX 10.13.5]
  • Monorepo [no]
  • How you are using Babel: [babel-jest]
@babel-bot
Copy link
Collaborator

Hey @jmikulka! We really appreciate you taking the time to report an issue. The collaborators
on this project attempt to help as many people as possible, but we're a limited number of volunteers,
so it's possible this won't be addressed swiftly.

If you need any help, or just have general Babel or JavaScript questions, we have a vibrant Slack
community that typically always has someone willing to help. You can sign-up here
for an invite.

@jmikulka
Copy link
Author

jmikulka commented Aug 9, 2018

Based on a slack discussion https://babeljs.slack.com/archives/C0DFJT81H/p1533733732000482 I've found a solution.

In deps.test.js replace

import * as deps from './deps';
const mockLogger = jest.fn();
jest.spyOn(deps, 'logger').mockImplementation(mockLogger);

with

const mockLogger = jest.fn();
jest.mock('./deps', () => ({ logger: mockLogger }));
const deps = require('./deps');

However, by mocking ./deps this way, we lose any other named/default exports.

@echjordan
Copy link

echjordan commented Oct 16, 2018

I'm getting the same error after upgrading to babel 7. In addition to this getter error, I'm getting an error that seems to be common when minified files are not properly separated by semicolons, (intermediate value).concat is not a function and wanted to mention here in case they're related.
I used babel-upgrade, have followed the documentation for upgrading.
Using:

    "@babel/core": "^7.1.2",
    "@babel/helper-plugin-utils": "^7.0.0",
    "@babel/plugin-proposal-class-properties": "^7.0.0",
    "@babel/plugin-proposal-decorators": "^7.1.2",
    "@babel/plugin-proposal-optional-chaining": "^7.0.0",
    "@babel/polyfill": "^7.0.0",
    "@babel/preset-env": "^7.1.0",
    "@babel/register": "^7.0.0"```

@echjordan
Copy link

Any update on this issue?

@beshanoe
Copy link

Encountered the same issue in my tests. I think, mocking the dependency is the only right approach, cause patching exported object is pretty unsafe and can lead to unexpected behaviour. So from that point of view it's good that babel 7 started enforcing this.
@jmikulka in order to not to lose other named exports you can do the following:

jest.mock("./deps", () => ({
  ...jest.requireActual("./deps"),
  logger: jest.fn()
}));

test("test something", () => {
  const deps = require("./deps");

  // do something

  expect(deps.logger).toHaveBeenCalled();
});

That's how I solved it in my tests :)

@nicolo-ribaudo
Copy link
Member

Yah that is a good solution. I'm closing this issue because it's not a bug; the specification mandates that esmodules may not be mutated from the outside.

@lock lock bot added the outdated A closed issue/PR that is archived due to age. Recommended to make a new issue label Mar 15, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Mar 15, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
outdated A closed issue/PR that is archived due to age. Recommended to make a new issue
Projects
None yet
Development

No branches or pull requests

5 participants