Skip to content

fix the illegal syntax in a module for test#3193

Merged
TheLarkInn merged 2 commits intowebpack:masterfrom
ts-webpack:fix-1792
Oct 27, 2016
Merged

fix the illegal syntax in a module for test#3193
TheLarkInn merged 2 commits intowebpack:masterfrom
ts-webpack:fix-1792

Conversation

@e-cloud
Copy link
Copy Markdown
Contributor

@e-cloud e-cloud commented Oct 26, 2016

Please check if the PR fulfills these requirements

  • Tests for the changes have been added (for bug fixes )

What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)

  • Bugfix for tests

What is the current behavior? (You can also link to an open issue here)
#1792
The test case for harmony-export-precedence has illegal syntax.

What is the new behavior?
no syntax error

Does this PR introduce a breaking change?

  • Yes
  • No

@sokra
Copy link
Copy Markdown
Member

sokra commented Oct 26, 2016

Why is this illegal?

@e-cloud
Copy link
Copy Markdown
Contributor Author

e-cloud commented Oct 26, 2016

you can review that issue

@sokra
Copy link
Copy Markdown
Member

sokra commented Oct 26, 2016

Ah ok, so it makes sense to display a warning or a Syntax error in this case?

@e-cloud
Copy link
Copy Markdown
Contributor Author

e-cloud commented Oct 26, 2016

The error will be reported by acorn(the latest version).(There is another PR update the deps later)

The point is should we add testcase to detect the error.

@e-cloud e-cloud mentioned this pull request Oct 26, 2016
10 tasks
this is related to webpack#1792, after removing the ilegal code,
the code still cover the cases except case 3.
@TheLarkInn
Copy link
Copy Markdown
Member

Looks good. Test failing on AppVeyor is timeout exceeding. Created #3198 for it.

@TheLarkInn TheLarkInn merged commit 4846d29 into webpack:master Oct 27, 2016
@e-cloud e-cloud deleted the fix-1792 branch October 29, 2016 13:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants