Skip to content
This repository has been archived by the owner on Sep 16, 2023. It is now read-only.

Lodash is set to undefined in build if its loaded onto a local 'this' #235

Open
usman-subhani opened this issue Jan 4, 2020 · 4 comments

Comments

@usman-subhani
Copy link

Issue occurs in this situation

import _ from 'lodash';

class test {
  constructor() {
    this._ = _; // lodash is replaced with 'undefined' here
  }
  someMethod() {
    const value = this._.get({a:1}, 'a'); // error thrown here because this._ is undefined
  }
}
@jdalton
Copy link
Member

jdalton commented Jan 7, 2020

Hi @usman-subhani!

Can you post what the transpiled code looks like?

@usman-subhani
Copy link
Author

usman-subhani commented Jan 7, 2020

Hey @jdalton, sure. This is the transpiled code with the plugin

"use strict";
__webpack_require__.r(__webpack_exports__);
/* harmony export (binding) */ __webpack_require__.d(__webpack_exports__, "test", function() { return test; });
class test {
  constructor() {
    this._ = undefined; // lodash is replaced with 'undefined' here
  }

  someMethod() {
    const value = this._.get({
      a: 1
    }, 'a'); // error thrown here because this._ is undefined

  }

}

and this is the code after removing the plugin

/* harmony export (binding) */ __webpack_require__.d(__webpack_exports__, "test", function() { return test; });
/* harmony import */ var lodash__WEBPACK_IMPORTED_MODULE_0__ = __webpack_require__(/*! lodash */ "./node_modules/lodash/lodash.js");
/* harmony import */ var lodash__WEBPACK_IMPORTED_MODULE_0___default = /*#__PURE__*/__webpack_require__.n(lodash__WEBPACK_IMPORTED_MODULE_0__);

class test {
  constructor() {
    this._ = lodash__WEBPACK_IMPORTED_MODULE_0___default.a; // lodash is imported and set here
  }

  someMethod() {
    const value = this._.get({
      a: 1
    }, 'a'); // works fine now

  }

}

This is the .babelrc config

{
  "presets": [
    "@babel/preset-react",
    ["@babel/preset-env", {
      "targets": {
        "chrome": "72",
        "firefox": "64",
        "opera": "50",
        "node": "current"
      }
    }]
  ],
  "plugins": ["lodash", "@babel/plugin-transform-runtime"]
}

I have v3.3.4 of babel-plugin lodash

@drudv
Copy link

drudv commented Sep 8, 2020

Have the same issue: lodash is being replaced with undefined. If I disable babel-plugin-lodash, the reference is preserved.

@JackieCheung
Copy link

Have the same issue: lodash is being replaced with undefined. If I disable babel-plugin-lodash, the reference is preserved.

same issue, is there a solution now?

keyurparalkar added a commit to appsmithorg/appsmith that referenced this issue Jun 19, 2023
## Description
This issue is happening because we are getting lodash library as
`undefined` when we pass it to` derived.value` function here. This is
happening because of the following reason:
* [Bundle optimisation
PR](#21667) introduces a new
plugin called: `babel-plugin-lodash`. It helps in transforming the
imports.
* But this plugin doesn't work on the following pattern: `import _ from
"lodash"`. Here it replaces `_` with `undefined`
[ This
file](https://github.com/appsmithorg/appsmith/blob/3bc50d9632cf269935aed6657092ddb8b7e8c92f/app/client/src/workers/common/JSLibrary/lodash-wrapper.js#L1)
explains the exact scenario that we are facing.
There is also [an open
bug](lodash/babel-plugin-lodash#235) for this
issue on the `babel-plugin-lodash` repository.

This PR imports lodash from the lodash-wrapper instead of directly
importing as follows: `import _ from "lodash"`
>
#### PR fixes following issue(s)
Fixes #23671

#### Type of change
- Bug fix (non-breaking change which fixes an issue)

## Testing
>
#### How Has This Been Tested?

- [x] Manual
- [ ] Jest
- [x] Cypress
- should check that value entered in currency field appears in the
actual field
>
>
#### Test Plan
> Add Testsmith test cases links that relate to this PR
>
>
#### Issues raised during DP testing
> Link issues raised during DP testing for better visiblity and tracking
(copy link from comments dropped on this PR)
>
>
>
## Checklist:
#### Dev activity
- [x] My code follows the style guidelines of this project
- [x] I have performed a self-review of my own code
- [ ] I have commented my code, particularly in hard-to-understand areas
- [ ] I have made corresponding changes to the documentation
- [ ] My changes generate no new warnings
- [ ] I have added tests that prove my fix is effective or that my
feature works
- [ ] New and existing unit tests pass locally with my changes
- [ ] PR is being merged under a feature flag


#### QA activity:
- [ ] [Speedbreak
features](https://github.com/appsmithorg/TestSmith/wiki/Guidelines-for-test-plans#speedbreakers-)
have been covered
- [ ] Test plan covers all impacted features and [areas of
interest](https://github.com/appsmithorg/TestSmith/wiki/Guidelines-for-test-plans#areas-of-interest-)
- [ ] Test plan has been peer reviewed by project stakeholders and other
QA members
- [ ] Manually tested functionality on DP
- [ ] We had an implementation alignment call with stakeholders post QA
Round 2
- [ ] Cypress test cases have been added and approved by SDET/manual QA
- [ ] Added `Test Plan Approved` label after Cypress tests were reviewed
- [ ] Added `Test Plan Approved` label after JUnit tests were reviewed
keyurparalkar added a commit to appsmithorg/appsmith that referenced this issue Jun 20, 2023
## Description
This issue is happening because we are getting lodash library as
`undefined` when we pass it to` derived.value` function here. This is
happening because of the following reason:
* [Bundle optimisation
PR](#21667) introduces a new
plugin called: `babel-plugin-lodash`. It helps in transforming the
imports.
* But this plugin doesn't work on the following pattern: `import _ from
"lodash"`. Here it replaces `_` with `undefined`
[ This
file](https://github.com/appsmithorg/appsmith/blob/3bc50d9632cf269935aed6657092ddb8b7e8c92f/app/client/src/workers/common/JSLibrary/lodash-wrapper.js#L1)
explains the exact scenario that we are facing.
There is also [an open
bug](lodash/babel-plugin-lodash#235) for this
issue on the `babel-plugin-lodash` repository.

This PR imports lodash from the lodash-wrapper instead of directly
importing as follows: `import _ from "lodash"`
>
#### PR fixes following issue(s)
Fixes #23671

#### Type of change
- Bug fix (non-breaking change which fixes an issue)

## Testing
>
#### How Has This Been Tested?

- [x] Manual
- [ ] Jest
- [x] Cypress
- should check that value entered in currency field appears in the
actual field
>
>
#### Test Plan
> Add Testsmith test cases links that relate to this PR
>
>
#### Issues raised during DP testing
> Link issues raised during DP testing for better visiblity and tracking
(copy link from comments dropped on this PR)
>
>
>
## Checklist:
#### Dev activity
- [x] My code follows the style guidelines of this project
- [x] I have performed a self-review of my own code
- [ ] I have commented my code, particularly in hard-to-understand areas
- [ ] I have made corresponding changes to the documentation
- [ ] My changes generate no new warnings
- [ ] I have added tests that prove my fix is effective or that my
feature works
- [ ] New and existing unit tests pass locally with my changes
- [ ] PR is being merged under a feature flag

#### QA activity:
- [ ] [Speedbreak
features](https://github.com/appsmithorg/TestSmith/wiki/Guidelines-for-test-plans#speedbreakers-)
have been covered
- [ ] Test plan covers all impacted features and [areas of
interest](https://github.com/appsmithorg/TestSmith/wiki/Guidelines-for-test-plans#areas-of-interest-)
- [ ] Test plan has been peer reviewed by project stakeholders and other
QA members
- [ ] Manually tested functionality on DP
- [ ] We had an implementation alignment call with stakeholders post QA
Round 2
- [ ] Cypress test cases have been added and approved by SDET/manual QA
- [ ] Added `Test Plan Approved` label after Cypress tests were reviewed
- [ ] Added `Test Plan Approved` label after JUnit tests were reviewed

(cherry picked from commit f47b1c8)
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

No branches or pull requests

4 participants