Skip to content

No-op string in constructor is repeated and reordered when constructor assignment is used #48671

Closed
#48687
Closed
No-op string in constructor is repeated and reordered when constructor assignment is used#48671
#48687
Assignees
jakebailey
Labels
BugA bug in TypeScriptFix AvailableA PR has been opened for this issue
Milestone
TypeScript 4.6.5

Description

@rhuitl
Robert Huitl (rhuitl)
opened on Apr 13, 2022

Bug Report

The TypeScript code

class Example {
    constructor(private A: string) {
        "ngInject";
    }
}

when compiled with TS 4.6.2 yields:

"use strict";
class Example {
    constructor(A) {
        this.A = A;
        "ngInject";
        "ngInject";
    }
}

when compiled with TS 4.5.5 yields:

"use strict";
class Example {
    constructor(A) {
        "ngInject";
        this.A = A;
    }
}

The differences are:

  • the string "ngInject;" is repeated
  • the string appears after the constructor assignment, not before as in 4.5.

For our use case, the repetition is probably not a real problem, however the re-ordering is. If any of these two are by design now, we can work around - but I wanted to double check with the TypeScript developers because it seems to be a bit unexpected.

🕗 Version & Regression Information

We observed errors in a AngularJS-based project using ng-annotate-patched which relies on the string to annotate classes for AngularJS. It works wiht TS 4.5 and broke with TS 4.6.

TS 4.6.2 (broken)
TS 4.5.5 (good)

⏯ Playground Link

https://www.typescriptlang.org/play?removeComments=true&target=2&ts=4.5.5#code/MYGwhgzhAECiAeYC2AHEBTaBvAUASGAHsA7CAFwCcBXYMwigChQoEsA3MMzAQQC5pyrYgHMAlNnx4ARCICSxAFbpaUgNz4Avji1A

Activity

  1. Ryan Cavanaugh (RyanCavanaugh)
    added
    BugA bug in TypeScript
    on Apr 13, 2022
  2. Ryan Cavanaugh (RyanCavanaugh)
    assigned
    Jake Bailey (jakebailey)
    on Apr 13, 2022
  3. Ryan Cavanaugh (RyanCavanaugh)
    added this to the TypeScript 4.7.1 milestone on Apr 13, 2022

  4. RyanCavanaugh commented on Apr 13, 2022

    @RyanCavanaugh
    Ryan Cavanaugh (RyanCavanaugh)
    on Apr 13, 2022
    Member

    👉 Josh Ghoulberg 👻 (@JoshuaKGoldberg) if you feel like it

  5. jakebailey commented on Apr 13, 2022

    @jakebailey
    Jake Bailey (jakebailey)
    on Apr 13, 2022
    Member

    The repetition is weird, but the reordering doesn't seem to be wrong to me at first glance; I would think that the parameters being properties implies that they must be assigned before any part of the body executes.

    Is there some special case for these sort of string-y statements at the start of blocks? Playground Link

    I guess so, because "use strict".

  6. JoshuaKGoldberg commented on Apr 13, 2022

    @JoshuaKGoldberg
    Josh Ghoulberg 👻 (JoshuaKGoldberg)
    on Apr 13, 2022
    Contributor

    Yes, they're directive prologues, also sometimes callied prologue directives. I've only ever seen "use esm" and "use strict" in the wild till now. TIL about ng-annotate-patched.

    #29374 and https://blog.joshuakgoldberg.com/code-before-super-technical-overview/ have a good amount of backing context for how those are meant to be at the beginning of the constructor.

    I can probably take a look within this week!

    Edit: or today 😄

  7. jakebailey commented on Apr 13, 2022

    @jakebailey
    Jake Bailey (jakebailey)
    on Apr 13, 2022
    Member

    Also TIL that the spec lets you have whatever prologues you want. 😄

  8. fatcerberus commented on Apr 13, 2022

    @fatcerberus
    Bruce Pascoe (fatcerberus)
    on Apr 13, 2022

    I mean, why wouldn't it? They're just strings, if the engine doesn't recognize a directive, it can just ignore it as a no-op.

  9. jakebailey commented on Apr 13, 2022

    @jakebailey
    Jake Bailey (jakebailey)
    on Apr 13, 2022
    Member

    I just mean from the POV that there's nothing wrong with this code:

    function foo() {
    console.log("Hello, world!");
    console.log(Math.random());
    "some random string as an expression statement";
}

    Because a string is an expression which can be alone in a statement.

    But if it's the first statement of a source file or a function body, then it's special and must not be reordered even if our emit is going to be placing things semantically "before" the block the user typed, because that changes its meaning.

  10. jakebailey commented on Apr 13, 2022

    @jakebailey
    Jake Bailey (jakebailey)
    on Apr 13, 2022
    Member

    FWIW this bug applies with "use strict" too (even though for some reason, running this emitted code crashes; running it elsewhere does not).

    Playground Link

    https://glot.io/snippets/g8u41v3925

  11. fatcerberus commented on Apr 13, 2022

    @fatcerberus
    Bruce Pascoe (fatcerberus)
    on Apr 13, 2022

    Out of curiosity, does the spec say anything about multiple directives? i.e. if I write...

    function foo() {
    "use fooey";
    "use bard";
    console.log(812);
}

    ...are both strings required to be kept at the top of the function?

  12. jakebailey commented on Apr 13, 2022

    @jakebailey
    Jake Bailey (jakebailey)
    on Apr 13, 2022
    Member

    I don't know about the spec, but at runtime it sure seems like it applies all of them: https://glot.io/snippets/g8u48lkc4o (don't ask me what runtime this site uses)

  13. TypeScript Bot (typescript-bot)
    added
    Fix AvailableA PR has been opened for this issue
    on Apr 13, 2022

  14. JoshuaKGoldberg commented on Apr 13, 2022

    @JoshuaKGoldberg
    Josh Ghoulberg 👻 (JoshuaKGoldberg)
    on Apr 13, 2022
    Contributor

    From https://es5.github.io/#x14.1:

    A Directive Prologue may contain more than one Use Strict Directive. However, an implementation may issue a warning if this occurs.

  15. fatcerberus commented on Apr 14, 2022

    @fatcerberus
    Bruce Pascoe (fatcerberus)
    on Apr 14, 2022

    Yes, but that's specifically only about "use strict":

    A Use Strict Directive is an ExpressionStatement in a Directive Prologue whose StringLiteral is either the exact character sequences "use strict" or 'use strict'.

    What I was wondering about is, if there are multiple custom directives other than use strict, if those are still considered part of the prologue. The spec seems a bit vague on that point.

    edit: Just read that section again, looks like the answer to my question is yes:

    A Directive Prologue is the longest sequence of ExpressionStatement productions occurring as the initial SourceElement productions of a Program or FunctionBody and where each ExpressionStatement in the sequence consists entirely of a StringLiteral token followed a semicolon. The semicolon may appear explicitly or may be inserted by automatic semicolon insertion. A Directive Prologue may be an empty sequence.

  16. Jake Bailey (jakebailey)
    closed this as completedin #48687on Apr 14, 2022

  17. rhuitl commented on Apr 14, 2022

    @rhuitl
    Robert Huitl (rhuitl)
    on Apr 14, 2022
    Author

    23 hours and 57 minutes from a bug report to a fix, you guys are awesome :-) Thank you!

  18. Daniel Rosenwasser (DanielRosenwasser)
    modified the milestones: TypeScript 4.7.1, TypeScript 4.6.5 on Apr 26, 2022
  19. Babak K. Shandiz (babakks)
    added a commit that references this issue on Jul 22, 2022

    🎭 Merge branch 'main' into fix-mapped-type-impl

    ae5ed13
  20. microsoft
    locked as resolved and limited conversation to collaborators on Oct 22, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

BugA bug in TypeScriptFix AvailableA PR has been opened for this issue

Type

No type

Projects

No projects

    Milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions