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

Optional chaining is not compiled correctly #760

Closed
jansedlon opened this issue May 4, 2020 · 19 comments · Fixed by #762
Closed

Optional chaining is not compiled correctly #760

jansedlon opened this issue May 4, 2020 · 19 comments · Fixed by #762
Labels
Milestone

Comments

@jansedlon
Copy link

I tried to use SWC for a quite big project... Around 130k LOC. In a code, there an attempt to use optional chaining to window object and its key. However when I try to compile it, it throws an error that ref1 is undefined. Rust probably tries to use some variable that it did not created.

Source code
const Auth = window?.['aws-amplify']?.Auth

Compiled code
var Auth = window === null || window === void 0 ? void 0 : window['aws-amplify'] === null || ref1 === void 0 ? void 0 : window['aws-amplify'].Auth;

Config

{
  "jsc": {
    "parser": {
      "syntax": "ecmascript",
      "jsx": true,
      "numericSeparator": false,
      "classPrivateProperty": false,
      "privateMethod": false,
      "classProperty": true,
      "functionBind": false,
      "optionalChaining": true,
      "dynamicImport": true,
      "nullishCoalescing": true,
      "decorators": true,
      "decoratorsBeforeExport": true
    },
    "target": "es2015",
    "loose": false,
    "transform": {
      "react": {
        "pragma": "React.createElement",
        "pragmaFrag": "React.Fragment",
        "throwIfNamespace": true,
        "development": false,
        "useBuiltins": false
      },
      "optimizer": {
        "globals": {
          "vars": {
            "__DEBUG__": "true"
          }
        }
      }
    }
  }
}

Expected behavior
Working optional chaining without undefined variable

Version
The version of @swc/core: 1.1.40

@jansedlon jansedlon added the C-bug label May 4, 2020
@jansedlon
Copy link
Author

Can I help somehow to speed up this fix? I'm not even intermediate in Rust however :/

@kdy1
Copy link
Member

kdy1 commented May 4, 2020

I'm working on this, but I failed to reproduce the issue.
Can you share some more code?

---- issue_760_1 stdout ----
>>>>> Orig <<<<<
const Auth = window?.['aws-amplify']?.Auth
----- Actual -----
>>>>> Code <<<<<
var ref;
const Auth = window === null || window === void 0 ? void 0 : (ref = window['aws-amplify']) === null || ref === void 0 ? void 0 : ref.Auth;

thread 'issue_760_1' panicked at 'assertion failed: `(left == right)`
            var ref;
const Auth = window === null || window === void 0 ? void 0 : (ref = window['aws-amplify']) === null || ref === void 0 ? void 0 : ref.Auth;
---- issue_760_2 stdout ----
>>>>> Orig <<<<<
const Auth = window?.aws?.Auth
----- Actual -----
>>>>> Code <<<<<
var ref;
const Auth = window === null || window === void 0 ? void 0 : (ref = window.aws) === null || ref === void 0 ? void 0 : ref.Auth;

thread 'issue_760_2' panicked at 'assertion failed: `(left == right)`
            var ref;
const Auth = window === null || window === void 0 ? void 0 : (ref = window.aws) === null || ref === void 0 ? void 0 : ref.Auth;
---- issue_760_3 stdout ----
>>>>> Orig <<<<<
const ref = 1; const Auth = window?.aws?.Auth
----- Actual -----
>>>>> Code <<<<<
var ref;
const ref1 = 1;
const Auth = window === null || window === void 0 ? void 0 : (ref = window.aws) === null || ref === void 0 ? void 0 : ref.Auth;

thread 'issue_760_3' panicked at 'assertion failed: `(left == right)`
            var ref;
const ref1 = 1;
const Auth = window === null || window === void 0 ? void 0 : (ref = window.aws) === null || ref === void 0 ? void 0 : ref.Auth;

kdy1 added a commit to kdy1/swc that referenced this issue May 4, 2020
@jansedlon
Copy link
Author

So.. One thing is that I have this aws-amplify defined in webpack resolve and externals like this..

resolve: {
    extensions: ["*", ".js", ".jsx"],
    alias: {
      moment: "window.moment",
      jQuery: "window.jQuery",
      jquery: "window.jQuery",
      "aws-amplify": 'window["aws-amplify"]',
      firebase: 'window["firebase"]',
    },
  },
  externals: [
    {
      moment: "moment",
      jQuery: "jQuery",
      jquery: "jQuery",
      "aws-amplify": 'window["aws-amplify"]',
      firebase: 'window["firebase"]',
    },
  ],

I also have aws-amplify defined in ProvidePlugin in webpack

new webpack.ProvidePlugin({
      "aws-amplify": "aws-amplify",
      'window["aws-amplify"]': "aws-amplify",
      'window["firebase"]': "firebase",
    }),

@jansedlon
Copy link
Author

I removed the last question mark and it works.. However ran into another problem with legacy decorators.. You don't support these, right?

@kdy1
Copy link
Member

kdy1 commented May 4, 2020

@jansedlon It's supported, but not documented yet. I verified that the legacy decorator works with the config below. I'll update the documentation.

{
  "jsc": {
    "parser": {
      "syntax": "ecmascript",
      "decorators": true
    },
    "transform": {
      "legacyDecorator": true
    }
  }
}

`

@jansedlon
Copy link
Author

jansedlon commented May 4, 2020

@kdy1 That's awesome, thank you! Let me know if you try it with that babel config and if you need more code.

@kdy1
Copy link
Member

kdy1 commented May 4, 2020

@jansedlon Thanks! Please feel free to file an issue when you encounter a bug.

@jansedlon
Copy link
Author

@kdy1 Hello, I've got update. Looks like that this happens in more places and might not be specifically optional-chaining issue..
While use Redux reducer in React, default value for function parameter throws the same error..

Original code

const initialState = {...}
export default function reducer(state = initialState, action = {}) {
    switch (action.type) {
        case SET:
            return ...
        case REPLACE:
            return ...
        case RESET_CURSOR:
            return ...
        case TOGGLE_BUSY: {
            return...
        }
        default:
            return state
    }
}

Compiled code
image

@kdy1
Copy link
Member

kdy1 commented May 5, 2020

At first glance, it seems like a bug of the resolver pass.
Currently, the resolver pass does not have special logic for references in parameter defaults, while it should.

I'll fix it asap.


I found that logic exists. It's a bug of the optimizer pass. Anyway, it seems like an easy bug.

@jansedlon
Copy link
Author

In a file where this reducer is defined are also some consts.. Which I can't find in the compiled source.. Looks like it has been just thrown out.

@jansedlon
Copy link
Author

jansedlon commented May 5, 2020

@kdy1 Thank you! Is there any way I can buy you https://www.buymeacoffee.com/ ?

@kdy1
Copy link
Member

kdy1 commented May 5, 2020

@jansedlon I created an account :)
https://www.buymeacoffee.com/kdy1
Thank you very much!

@kdy1 kdy1 added this to the v1.1.41 milestone May 5, 2020
@jansedlon
Copy link
Author

@kdy1 You have to setup payment method so I can send you something 😅

@kdy1
Copy link
Member

kdy1 commented May 5, 2020

@jansedlon I configured it. Thanks :)

kdy1 added a commit to kdy1/swc that referenced this issue May 5, 2020
kdy1 added a commit to kdy1/swc that referenced this issue May 5, 2020
kdy1 added a commit to kdy1/swc that referenced this issue May 5, 2020
kdy1 added a commit to kdy1/swc that referenced this issue May 5, 2020
kdy1 added a commit to kdy1/swc that referenced this issue May 5, 2020
@kdy1 kdy1 closed this as completed in #762 May 5, 2020
@kdy1
Copy link
Member

kdy1 commented May 5, 2020

Published swc/core@v1.1.41

@jansedlon
Copy link
Author

@kdy1 Great job! Looks like that previous problem is eliminated. However! 😅 Another problem with "dead code elimination"

Original code

import {
    INSTAGRAM_CHECK_PATTERN,
    RESOURCE_FACEBOOK,
    RESOURCE_INSTAGRAM,
    RESOURCE_WEBSITE,
} from '../../../../consts'

const resources = [
    {
        value: RESOURCE_WEBSITE,
        label: 'Webové stránky',
    },
    {
        value: RESOURCE_FACEBOOK,
        label: 'Facebook',
    },
    {
        value: RESOURCE_INSTAGRAM,
        label: 'Instagram',
    },
]

// export default decorated React class component

Compiled code

// About 15 empty lines


var resources = [
    {
        value: RESOURCE_WEBSITE,
        label: 'Webové stránky'
    },
    {
        value: RESOURCE_FACEBOOK,
        label: 'Facebook'
    },
    {
        value: _consts__WEBPACK_IMPORTED_MODULE_6__["RESOURCE_INSTAGRAM"],
        label: 'Instagram'
    }, 
];
var _dec = Object(_validator__WEBPACK_IMPORTED_MODULE_9__["i18n"])();

@jansedlon
Copy link
Author

I found that the variables are declared in the compiled code like this

/* harmony export (binding) */ __webpack_require__.d(__webpack_exports__, "RESOURCE_WEBSITE", function() { return RESOURCE_WEBSITE; });
/* harmony export (binding) */ __webpack_require__.d(__webpack_exports__, "RESOURCE_FACEBOOK", function() { return RESOURCE_FACEBOOK; });
/* harmony export (binding) */ __webpack_require__.d(__webpack_exports__, "RESOURCE_INSTAGRAM", function() { return RESOURCE_INSTAGRAM; });

However it looks like that RESOURCE_WEBSITE and RESOURCE_FACEBOOK are not imported in a way that RESOURCE_INSTAGRAM is

@jansedlon
Copy link
Author

Moved to #763

@swc-bot
Copy link
Collaborator

swc-bot commented Oct 28, 2022

This closed issue has been automatically locked because it had no new activity for a month. If you are running into a similar issue, please create a new issue with the steps to reproduce. Thank you.

@swc-project swc-project locked as resolved and limited conversation to collaborators Oct 28, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Development

Successfully merging a pull request may close this issue.

3 participants