Skip to content

Change error message for default export in namespace #11119

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.

Sign up for GitHub

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

Jump to bottom
Merged
mhegazy merged 19 commits into from
Dec 22, 2016
Merged

Change error message for default export in namespace #11119

mhegazy merged 19 commits into from
Dec 22, 2016

Conversation

@YuichiNukiyama
Copy link
Contributor

@YuichiNukiyama YuichiNukiyama commented Sep 24, 2016 •
edited by DanielRosenwasser
Loading

Fixes #11092

@YuichiNukiyama
Copy link
Contributor Author

YuichiNukiyama commented Nov 2, 2016

ping @DanielRosenwasser

RyanCavanaugh
RyanCavanaugh reviewed Nov 2, 2016
View reviewed changes
src/compiler/checker.ts Outdated
else if (source.symbol && source.flags & TypeFlags.ObjectType && globalObjectType === source) {
reportError(Diagnostics.The_Object_type_is_assignable_to_very_few_other_types_Did_you_mean_to_use_the_any_type_instead);
}
else if (source.symbol && source.flags & TypeFlags.ObjectType && globalObjectType === source) {
Copy link
Member

@RyanCavanaugh RyanCavanaugh Nov 2, 2016

Choose a reason for hiding this comment

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

Needs remerge?

DanielRosenwasser
DanielRosenwasser requested changes Nov 2, 2016
View reviewed changes
tests/baselines/reference/parserExportAssignment5.errors.txt Outdated
export = A;
~~~~~~~~~~~
!!! error TS1063: An export assignment cannot be used in a namespace.
!!! error TS1063: A default export can only be used in an ECMAScript-style module.
Copy link
Member

@DanielRosenwasser DanielRosenwasser Nov 2, 2016

Choose a reason for hiding this comment

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

This is a regression - the old error is actually more appropriate here.

@DanielRosenwasser
Copy link
Member

DanielRosenwasser commented Nov 18, 2016

@YuichiNukiyama would you still be interested in fixing up the bug? I think all you'd need to do is still issue the old error message for export =, but use the new one for export default

@YuichiNukiyama
Copy link
Contributor Author

YuichiNukiyama commented Nov 19, 2016 •
edited
Loading

@DanielRosenwasser Sorry for the delay. How is this?

@DanielRosenwasser
Copy link
Member

DanielRosenwasser commented Nov 24, 2016

Looks good! However, it looks like you've added a test from one of your older PRs (assigningFromObjecToAnythingElse). If you delete that I think we can pull this change in.

@DanielRosenwasser
Copy link
Member

DanielRosenwasser commented Nov 24, 2016

@YuichiNukiyama assigningFromObjecToAnythingElse.errors.txt is still there

@DanielRosenwasser
Copy link
Member

DanielRosenwasser commented Nov 25, 2016 •
edited
Loading

assigningFromObjecToAnythingElse.ts should still be removed and I would think asiPublicPrivateProtected.ts should be restored.

@YuichiNukiyama
Copy link
Contributor Author

YuichiNukiyama commented Nov 25, 2016

@DanielRosenwasser fix it.

@mhegazy mhegazy closed this Dec 22, 2016
@mhegazy mhegazy reopened this Dec 22, 2016
@mhegazy mhegazy merged commit b227cf0 into microsoft:master Dec 22, 2016
@YuichiNukiyama YuichiNukiyama deleted the defult_export_error_message branch December 22, 2016 21:32
@microsoft microsoft locked and limited conversation to collaborators Jun 19, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@RyanCavanaugh RyanCavanaugh RyanCavanaugh left review comments

@DanielRosenwasser DanielRosenwasser DanielRosenwasser requested changes

+1 more reviewer

@mhegazy mhegazy mhegazy approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@YuichiNukiyama @DanielRosenwasser @RyanCavanaugh @mhegazy @msftclas