-
Notifications
You must be signed in to change notification settings - Fork 12.9k
Move type serialization to emitter #3690
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
Conversation
👍 can you guys update beta with this fix? |
@gdi2290 1.5 is basically stabilizing and not taking anything that's not extremely high priority. This will be going into 1.6. |
we found a workaround @DanielRosenwasser so no worries |
@@ -862,7 +862,7 @@ namespace ts { | |||
if (name.kind === SyntaxKind.Identifier) { | |||
let message = meaning === SymbolFlags.Namespace ? Diagnostics.Cannot_find_namespace_0 : Diagnostics.Cannot_find_name_0; | |||
|
|||
symbol = resolveName(name, (<Identifier>name).text, meaning, message, <Identifier>name); | |||
symbol = resolveName(name, (<Identifier>name).text, meaning, !ignoreErrors ? message : undefined, <Identifier>name); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i would just have htis as ignoreErrors ? undefined : message
I would say that this is very high priority and believe that it should go into 1.5. This is a critical bug that breaks any application using the reflection metadata api (yes experimental) and ES6 style exports. For example: export default class FriendsService {
names: Array<string>;
constructor() {
this.names = ["Alice", "Aarav", "Martín", "Shannon", "Ariana", "Kai"];
}
} import FriendsService from './FriendsService';
@Component({
...
appInjector: [FriendsService]
})
class DisplayComponent {
myName: string;
names: Array<string>;
constructor(friendsService: FriendsService) {
this.m
8000
yName = 'Alice';
this.names = friendsService.names;
}
} This example is taken directly from angular but I moved the two classes into separate files and used the ES6 See #3663 for another example In this case I would simply try to remove the type annotation on the constructor parameter, but that is required by Angular.... So either way it's broken. Therefore I make the argument that this is a critical bug and should be included in 1.5 |
@SnareChops sorry for the delay, it has been a relatively tricky change to get it. we will get you a build to unblock you asap. as for the release vehicle, i do not think 1.5 is an option. but we are working on getting a nightly release in place, so should have an "official" release with the fix soon. |
Move type serialization to emitter
Moves the logic for decorator metadata type serialization to the emitter.
This follows #3380, and also fixes #3663.