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

add guards support in JavascriptParser #15497

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

add guards support in JavascriptParser #15497

wants to merge 5 commits into from

Conversation

vankop
Copy link
Member

@vankop vankop commented Mar 9, 2022

What kind of change does this PR introduce?
feature
closes #14814

Did you add tests for your changes?
yes

Does this PR introduce a breaking change?
no

What needs to be documented once your changes are merged?
nothing

** notes regarding design **
add in parser.scope parser.scope.guards/parser.scope.inGuardPosition

termins:
guard - chain of ids that is safe to use ( is truthy ) in current parser.scope
in guard position- any possible guard ( chain of ids ) could be added to parser.scope.guards, this means that this guard will be safe to use in current and all children parser scopes. e.g. && logical expression:

import * as b from "b";
// x && y is a guard position since it is truthy only if both operands are truthy, also y could be guarded by x
// b is namespace, always safe to use
// b.a could be a guard
if (b && b.a) {
  if (c) b.a(); // b.a is a guard
}

also 2 binary expressions could be guard position:
x in y / x != y ( when x falsy )
lets assume that y is import specifier then:

import {y} from "y";

// y.x is a guard
"x" in y ? y.x() : null;

// y is a guard
if (null != y) {y();} // non strict equality is important since null !== y could be true with falsy y

also this could work with several guards positions, e.g. complex example:

import {a, b, c} from "a";

//  not in guard position, we support only &&
if ((x in a && b) || c) {
}

if ("x" in a && b || c) {
   a.x(); // is guarded
   b(); // not guarded
   c(); // not guarded
}

@webpack-bot
Copy link
Contributor

For maintainers only:

  • This needs to be documented (issue in webpack/webpack.js.org will be filed when merged)
  • This needs to be backported to webpack 4 (issue will be created when merged)

@vankop vankop mentioned this pull request Mar 9, 2022
@xarety
Copy link

xarety commented Nov 10, 2022

Hey guys, any updates? :)

Copy link
Member

@alexander-akait alexander-akait left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code looks good, because will be great to add more tests cases, I’m very afraid of regression, quite a serious change

@vankop
Copy link
Member Author

vankop commented Apr 12, 2024

@alexander-akait fixed

@alexander-akait
Copy link
Member

alexander-akait commented Apr 12, 2024

@vankop Looks like we don't have ?. optional chaining, right? And a["b"]?

@vankop
Copy link
Member Author

vankop commented Apr 12, 2024

optional chaining is not supported here.. in general we dont support optional chaining in import specifiers now as I remember, we just opt out optimizations. e.g.

import { a } from 'module';
a.b?.c;
// we optimize only "a.b" so if "b" is a namespace we import all namespace

@vankop
Copy link
Member Author

vankop commented Apr 12, 2024

I guess same for computed part of members expression like a["b"]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Ready for Merge
Development

Successfully merging this pull request may close these issues.

webpack should not warn import usage, when import is unreachable (e.g. guarded by if statement)
4 participants