Skip to content

add guards support in JavascriptParser#15497

Closed
vankop wants to merge 5 commits intomainfrom
feature/guards
Closed

add guards support in JavascriptParser#15497
vankop wants to merge 5 commits intomainfrom
feature/guards

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
@vankop vankop force-pushed the feature/guards branch 3 times, most recently from 6ea8dfa to 967da80 Compare March 10, 2022 09:14
@vankop vankop force-pushed the feature/guards branch 2 times, most recently from 01cf4ba to d9bd4ba Compare March 17, 2022 11:18
@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)

5 participants