Skip to content

fix(define): preserve non-optional property access past undefined member - #22016

Closed
Pcmhacker-piro wants to merge 1 commit into
webpack:mainfrom
Pcmhacker-piro:fix/define-undefined-member-chain
Closed

fix(define): preserve non-optional property access past undefined member#22016
Pcmhacker-piro wants to merge 1 commit into
webpack:mainfrom
Pcmhacker-piro:fix/define-undefined-member-chain

Conversation

@Pcmhacker-piro

@Pcmhacker-piro Pcmhacker-piro commented Sep 8, 2026 •
edited by coderabbitai Bot
Loading

Copy link
Copy Markdown
Contributor

Summary

Follow-up to #21827. In #21827, callMemberChain was updated using dropTrailingMembers so that a non-optional call past an undefined member (e.g. OBJ.MISSING.a?.()) emits undefined.a?.() and throws at runtime as native JavaScript would.

However, expressionMemberChain was not updated and still unconditionally replaced the entire member expression with undefined. Consequently, non-optional property accesses (such as OBJECT.SUB.UNKNOWN.a, OBJECT.SUB.UNKNOWN["a"], OBJECT.SUB.UNKNOWN.a.b, and OBJECT.SUB.UNKNOWN.a?.b) silently evaluated to undefined instead of throwing a TypeError when reading off undefined.

This PR brings the same chain-cutting logic to parser.hooks.expressionMemberChain:

  1. If the member is the leaf of the access chain (members.length - 1 === undefinedIndex) or the next link is optional (membersOptionals[undefinedIndex + 1]), the expression evaluates to undefined.
  2. Otherwise, dropTrailingMembers cuts the chain at undefinedIndex, emitting (undefined).a so that the non-optional property read executes against undefined and throws the expected TypeError.

Fixes #22014.

What kind of change does this PR introduce?

fix

Did you add tests for your changes?

Yes:

  • Added coverage for non-optional property reads (.a, ["a"], deep chained .a.b, and .a?.b) throwing in test/configCases/plugins/define-plugin-optional-chaining/index.js.
  • Updated test/configCases/plugins/define-plugin/index.js to assert undefined.A and verify it throws on execution.

Does this PR introduce a breaking change?

No. Affected expressions previously evaluated to undefined instead of throwing the runtime TypeError mandated by ECMAScript semantics.

If relevant, what needs to be documented once your changes are merged or what have you already documented?

n/a

Use of AI

No.

Summary by CodeRabbit

  • Bug Fixes
    • Improved handling of unknown member access in DefinePlugin expressions.
    • Non-optional property access now remains intact, preserving expected runtime errors when evaluated.
    • Optional chaining correctly resolves to undefined when an earlier member is unknown.
    • Added coverage for dot notation, computed properties, chained access, and mixed optional/non-optional expressions.

@Pcmhacker-hero
fix(define): preserve non-optional property access past undefined member
0c279a8 
When an object member defined via DefinePlugin resolves to undefined,
accessing properties non-optionally past that member was unconditionally
collapsed into undefined by expressionMemberChain.

At runtime, reading a property off undefined must throw a TypeError.
We now cut the chain at the undefined member using dropTrailingMembers
when the access is not optional, emitting (undefined).a so the
runtime TypeError is preserved natively.

Fixes webpack#22014
@linux-foundation-easycla

linux-foundation-easycla Bot commented Sep 8, 2026

Copy link
Copy Markdown

CLA Not Signed

@changeset-bot

changeset-bot Bot commented Sep 8, 2026

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: 0c279a8

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@github-actions

github-actions Bot commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

👋 Hi there! This PR was automatically flagged and closed by our quality checks.

If you believe this was a mistake, please review our contributing guidelines
and leave a comment explaining why this is a mistake.

@github-actions github-actions Bot closed this Sep 8, 2026
@coderabbitai

coderabbitai Bot commented Sep 8, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Review Change StackReview Change Stack

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: 808fe313-ddcf-4593-b48c-0e41aa8143ba

📥 Commits

Reviewing files that changed from the base of the PR and between c4cd6ca and 0c279a8.

📒 Files selected for processing (3)
  • lib/DefinePlugin.js
  • test/configCases/plugins/define-plugin-optional-chaining/index.js
  • test/configCases/plugins/define-plugin/index.js
📝 Walkthrough

Walkthrough

DefinePlugin now preserves runtime errors for non-optional accesses after undefined members while retaining optional-chain short-circuiting. Tests cover dot, computed, chained, and mixed member access.

Changes

DefinePlugin member access

Layer / File(s) Summary
Member-chain replacement and validation
lib/DefinePlugin.js, test/configCases/plugins/define-plugin-optional-chaining/index.js, test/configCases/plugins/define-plugin/index.js		 The plugin truncates non-optional chains at the undefined member and replaces optional chains with undefined. Tests verify emitted expressions, thrown errors, and optional-chain results.

Suggested reviewers: alexander-akait

Change: Bug fix · Severity of issue fixed: Medium

✨ Finishing Touches 💡 1
🛠️ Fix failing CI checks 💡
  • Create stacked PR
  • Commit on current branch

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@Pcmhacker-piro

Pcmhacker-piro commented Sep 8, 2026

Copy link
Copy Markdown
Contributor Author

Hi @webpack-maintainers, this PR was automatically flagged and closed by the quality checks bot.

This is a valid, focused bug fix for #22014 resolving non-optional property access in expressionMemberChain (following up on #21827). It includes full test coverage in test/configCases/plugins/define-plugin-optional-chaining/index.js and test/configCases/plugins/define-plugin/index.js, all tests pass locally, and a changeset has been added.

Could a maintainer please review and reopen this PR?

@Pcmhacker-piro

Pcmhacker-piro commented Sep 8, 2026

Copy link
Copy Markdown
Contributor Author

Superseded by @alexander-akait's official maintainer PR #22017 which resolves #22014.

@Pcmhacker-piro Pcmhacker-piro mentioned this pull request Sep 8, 2026
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

DefinePlugin: non-optional property access past an undefined member incorrectly evaluates to undefined instead of throwing TypeError

2 participants

@Pcmhacker-piro @Pcmhacker-hero