Optional chain is incorrectly removed when used on import

[jtbandes]

Bug report

What is the current behavior?

Webpack incorrectly compiles x?.value to (something).value when x is something that's been imported.

If the current behavior is a bug, please provide the steps to reproduce.

webpack.config.js:

module.exports = {
    entry: "./index.js",
    target: "web",
};

index.js:

import x from './x.js';
console.log(`optional value is: ${x?.value}`);

x.js:

export default undefined;

Produces dist/main.js:

(()=>{"use strict";console.log(`optional value is: ${(void 0).value}`)})();

which leads to TypeError: Cannot read property 'value' of undefined at runtime.

What is the expected behavior?

The ? should be retained or converted to a ternary operator.

When import x from './x.js'; is removed and replaced with const x = (() => void console)(); (something complicated enough that webpack doesn't completely constant-fold it), the output is:

(()=>{const o=void console;console.log(`optional value is: ${o?.value}`)})();

which is as expected (the ? is retained).

Other relevant information:
webpack version: latest, 5.27.2
Node.js version: v15.9.0
Operating System: macOS 11.2.2

[sojitramatang]

I am also facing the same issue.
When will this issue be resolved?
Any idea or workaround for this?

Thank you.

[rossng]

I'm also seeing this problem.

[alexander-akait]

Other example #13792

[jtbandes]

@vankop @sokra It looks like you two were the original authors on #11221 so I hope you can help — I spent some time understanding the root cause and looking at potential fixes, but I got stuck. It looks like this issue has been present since the initial PR and is not a regression.

The root cause is that a parser code path of extractMemberExpressionChain + objectAndMembersToName loses the optional information from property/call expressions and converts the identifier name to x.value:

webpack/lib/javascript/JavascriptParser.js

Line 106 in 911ec1a

name = name + "." + membersReversed[i];

The reason it only happens for imports is that the code path only applies when getFreeInfoFromVariable returns true:

webpack/lib/javascript/JavascriptParser.js

Lines 3478 to 3486 in 911ec1a

	const result = this.getFreeInfoFromVariable(rootName);
	if (!result) return undefined;
	const { info: rootInfo, name: resolvedRoot } = result;
	return {
	type: "expression",
	name: objectAndMembersToName(resolvedRoot, members),
	rootInfo,
	getMembers: memorize(() => members.reverse())
	};

One approach to solve this is changing extractMemberExpressionChain to return {name: string, optional: boolean}[] instead of just string[]. Then objectAndMembersToName can correctly keep the ?s, but I ran into an issue with this test where the new identifier name "_VALUE_?._PROP_" is incorrectly used as a key in the hookMap for hooks.evaluateIdentifier, so I imagine we would need to change the hook map as well. Changing this function signature also requires a public API change which might be disruptive.

Another approach I tried is changing extractMemberExpressionChain to return undefined if the chain contains any optional property accesses. This is also a public API change (albeit a smaller one), and also breaks expression evaluation as well as some other tests I haven't looked at in detail.

It seems like a larger refactor might be needed, unless you can recommend another solution that someone like me (with no prior experience contributing to webpack) would be able to do.

[hipstersmoothie]

@alexander-akait @sokra Any suggested workarounds for this issue?

[Cherry]

A simple workaround we've found is referencing the imported module as another variable. For example:

import x from './x.js';
const xRef = x;
console.log(`optional value is: ${xRef?.value}`);

Not ideal, but it works until this can be fixed.

[hipstersmoothie]

Yeah i've resorted to doing something similar, it's just a large codebase and i'm not certain i've got em all

[joeblynch]

I also ran into this issue. To avoid this issue cropping back up before a fix is implemented, I ended up forcing optional chaining to be transformed by babel before webpack has a chance to remove it. Obviously this removes the actual optional chaining operators in the bundled code, but at least it's functionally equivalent.

babel.config.js:

module.exports = {
  presets: [
    [
      '@babel/preset-env',
      {
        // force optional chaining transform
        include: ['proposal-optional-chaining'],
        // omitting the rest of my preset-env config for brevity
      }
    ],
  ]
};

[rossng]

Snowpack users are likely to run into this issue, and we came up with a similar solution to this one to configure Snowpack's Webpack plugin.

[
            "@snowpack/plugin-webpack",
            {
                extendConfig: (config) => {
                    const targetPlugin = config.module.rules.reduce(
                        (acc, rule) => acc ?? rule.use.find((plugin) => plugin.loader.includes("babel-loader")),
                        undefined
                    );
                    if (!targetPlugin) {
                        throw new Error("Could not find babel-loader plugin!");
                    }
                    targetPlugin.options.plugins = [require.resolve("@babel/plugin-proposal-optional-chaining")];

                    return config;
                },
            },
        ],

I didn't realise the include option existed though - that might let us simplify this a bit.

[JMS-1]

I'm not sure if this is the same issue but maybe it is so I will not open another issue. Although I'm using no babel but only ts-loader I get the same behaviour.

const id = this.props.id || store.data[0]?._id || ''

Is transpiled to

const e = this.props.id || n.data[0]._id || "";

Loosing the optional chaining will generate some run-time error if the array is empty.

I created a minimal reproduction here.

A fix of the issue would be of great help!

Thanks

Jochen

[vankop]

this is not fixed yet

[JMS-1]

Well I understand this has been merged into the main branch just an hour ago and will be available with the next release 5.64.5. Did you use this in your test? In fact nothing changed in 5.64.4 but in my opinion this is as expected.

[vankop]

Well I understand this has been merged into the main branch just an hour ago

thats not a fix, only part of it

[alexander-akait]

@vankop Can we close due #14784?

[vankop]

No, this issue about supporting
a.js

export * as c from './c.js'

index.js

import * as a from './a.js'
a.c?.b // or
a?.c?.b

// right now will produce
a.c.b
a.c.b

to produce optional chain in output we need outputOptions.environment.supportOptionalChaining

[JMS-1]

With 5.65.0 and

output: { ..., environment: { optionalChaining: true } },

const id = this.props.id || store.data[0]?._id || ''

Is still transpiled to

const e = this.props.id || n.data[0]._id || "";

when targeting ES2020.

[alexander-akait]

@JMS-1 I think your problem is not related to webpack, do you use babel?

[Jack-Works]

to produce optional chain in output we need outputOptions.environment.supportOptionalChaining

If webpack encountered ?. (which means not transpiled by babel), we should emit an error if outputOptions.environment.supportOptionalChaining is not true because webpack will not do any transpiling.

[JMS-1]

@JMS-1 I think your problem is not related to webpack, do you use babel?

No the problem ist that the ?. is removed bevore it event gets to the ts-loader.

const path = require('path')

module.exports = {
    entry: { index: path.join(__dirname, './src/index.ts') },
    mode: 'production',
    module: { rules: [{ test: /\.(ts|tsx)$/, use: 'ts-loader' }] },
    output: { filename: '[name].js', path: path.join(__dirname, 'build'), environment: { optionalChaining: true } },
    resolve: { extensions: [".js", '.ts'] },
}

[JMS-1]

Sorry, possible silly question: how do I define supportOptionalChaining?

[alexander-akait]

It is not yet implemented, work before is just stage to prepare fix

[GCSBOSS]

Just in case someone (like me) thought this was fixed on 5.65.0. It is still not fixed.

[vankop]

solved in main branch

[ekilah]

seems to be included in 5.71.0 now 🥳

[JMS-1]

5.71.0 now works even without environment: { optionalChaining: true }. Thanks!

[vankop]

yes, this is solved in 5.71