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 optional chaining and nullish coalescing support #11198

Closed
wants to merge 3 commits into from

Conversation

vankop
Copy link
Member

@vankop vankop commented Jul 17, 2020

What kind of change does this PR introduce?

closes #11186
closes #10227

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

@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)

@@ -837,9 +837,9 @@ external \\"test\\" 42 bytes [built]"
`;

exports[`StatsTestCases should print correct stats for filter-warnings 1`] = `
"Hash: 2220740a2bcff63070922220740a2bcff63070922220740a2bcff63070922220740a2bcff63070922220740a2bcff63070922220740a2bcff63070922220740a2bcff63070922220740a2bcff63070922220740a2bcff63070922220740a2bcff63070922220740a2bcff63070922220740a2bcff63070922220740a2bcff6307092
"Hash: 73a3198368d23afbbd6573a3198368d23afbbd6573a3198368d23afbbd6573a3198368d23afbbd6573a3198368d23afbbd6573a3198368d23afbbd6573a3198368d23afbbd6573a3198368d23afbbd6573a3198368d23afbbd6573a3198368d23afbbd6573a3198368d23afbbd6573a3198368d23afbbd6573a3198368d23afbbd65
Copy link
Member Author

Choose a reason for hiding this comment

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

don't know why hashes changed

- add handleStrictEqualityComparison callback
- add handleAbstractEqualityComparison callback
- rework evaluateIdentifier interface
- add tests

if (!process.version.startsWith("v14")) return;

const value10 = "" ?? require("fail");
Copy link
Member Author

@vankop vankop Jul 18, 2020

Choose a reason for hiding this comment

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

Test is passing, but since we render "" ?? false need to find a way to load this tests in Node.js < 14. Maybe use babel before load or move this to configCases and handle there with babel.

});

it("should evaluate module.hot to nullish", () => {
if (module.hot ?? true) {
Copy link
Member Author

Choose a reason for hiding this comment

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

Should I move this to configCases since this does not work with TestCasesHot?

@@ -0,0 +1,5 @@
it("should handle optional members", () => {
expect(
module.hot?.accept((() => {throw new Error("fail")})())
Copy link
Member Author

Choose a reason for hiding this comment

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

Should I move this to configCases since this does not work with TestCasesHot?


it("should correctly render defined data #2", () => {
const val1 = _VALUE_?._PROP_?._DEFINED_;
const val2 = _VALUE_?._PROP_?._UNDEFINED_;
Copy link
Member Author

Choose a reason for hiding this comment

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

Should I keep this test case? Same problem

Test is passing, but since we render "" ?? false need to find a way to load this tests in Node.js < 14. Maybe use babel before load or move this to configCases and handle there with babel.

devtool: false,
target: "web",
plugins: [
new DefinePlugin({
Copy link
Member Author

Choose a reason for hiding this comment

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

this import leads to typing errors..

@webpack-bot
Copy link
Contributor

@vankop The most important CI builds failed. This way your PR can't be merged.

Please take a look at the CI results from travis (failure) and appveyor (failure) and fix these issues.

@jfbrennan
Copy link

Thanks for this! Looking forward to a merge so I can use vanilla javascript lol

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants