- 
          
- 
                Notifications
    You must be signed in to change notification settings 
- Fork 1.7k
ref(build): Update to TypeScript 3.8.3 #4491
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
          
     Merged
      
      
            lobsterkatie
  merged 2 commits into
  kmclb-new-bundling-process
from
kmclb-upgrade-to-typescript-3.8.3
  
      
      
   
  Feb 3, 2022 
      
    
                
     Merged
            
            ref(build): Update to TypeScript 3.8.3 #4491
                    lobsterkatie
  merged 2 commits into
  kmclb-new-bundling-process
from
kmclb-upgrade-to-typescript-3.8.3
  
      
      
   
  Feb 3, 2022 
              
            Conversation
  
    
      This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
      Learn more about bidirectional Unicode characters
    
  
  
    
    | size-limit report
 | 
              
                    AbhiPrasad
  
              
              approved these changes
              
                  
                    Feb 3, 2022 
                  
              
              
            
            
707436f    to
    4146840      
    Compare
  
    4146840    to
    8bdd1c4      
    Compare
  
    
    
  lobsterkatie 
      added a commit
      that referenced
      this pull request
    
      Feb 8, 2022 
    
    
  
    
  lobsterkatie 
      added a commit
      that referenced
      this pull request
    
      Mar 31, 2022 
    
    
  
    
  lobsterkatie 
      added a commit
      that referenced
      this pull request
    
      Mar 31, 2022 
    
    
  
    
  lobsterkatie 
      added a commit
      that referenced
      this pull request
    
      Mar 31, 2022 
    
    
  
    
  lobsterkatie 
      added a commit
      that referenced
      this pull request
    
      Apr 1, 2022 
    
    
  
    
  lobsterkatie 
      added a commit
      that referenced
      this pull request
    
      Apr 4, 2022 
    
    
  
    
  lobsterkatie 
      added a commit
      that referenced
      this pull request
    
      Apr 5, 2022 
    
    
  
    
  lobsterkatie 
      added a commit
      that referenced
      this pull request
    
      Apr 6, 2022 
    
    
  
    
  lobsterkatie 
      added a commit
      that referenced
      this pull request
    
      Apr 7, 2022 
    
    
  
    
  lobsterkatie 
      added a commit
      that referenced
      this pull request
    
      Apr 7, 2022 
    
    
  
  
    Sign up for free
    to join this conversation on GitHub.
    Already have an account?
    Sign in to comment
  
      
  Add this suggestion to a batch that can be applied as a single commit.
  This suggestion is invalid because no changes were made to the code.
  Suggestions cannot be applied while the pull request is closed.
  Suggestions cannot be applied while viewing a subset of changes.
  Only one suggestion per line can be applied in a batch.
  Add this suggestion to a batch that can be applied as a single commit.
  Applying suggestions on deleted lines is not supported.
  You must change the existing code in this line in order to create a valid suggestion.
  Outdated suggestions cannot be applied.
  This suggestion has been applied or marked resolved.
  Suggestions cannot be applied from pending reviews.
  Suggestions cannot be applied on multi-line comments.
  Suggestions cannot be applied while the pull request is queued to merge.
  Suggestion cannot be applied right now. Please check back later.
  
    
  
    
This updates the SDK to use Typescript 3.8.3, in order to be able to use type-only imports and exports. (These are needed for us to turn on
isolatedModules, which is in turn is needed for us to switch our build tool away fromtsc, since no other tool understands the relationship between files.)As a result of this change, a few of the browser integration tests needed to be fixed so that all promises were explicitly awaited, a point about which the linter in 3.8 complains.
This is a breaking change for anyone using TS 3.7.x (there's no one using TS < 3.7.x, since that's our current minimum). That said, though there are plenty of public projects on GH using TS 3.7 and
@sentry/xyz, if you restrict it to projects using TS 3.7 and@sentry/xyz6.x, all you get are forks of this very repo. Granted, this isn't every project ever, but it's likely decently representative of the fact that if you've upgraded our SDK, you've almost certainly upgraded TS as well. We're going to wait until v7 to release this change in any case, but that's an indication that it won't affect many people when we do.Ultimately we may end up upgrading all the way to 4.x, but that can be left for a future PR if so.[UPDATE] For the foreseeable future, we're going to stick with 3.8.3. Though moving up to 4.x doesn't seem like it would affect many customers (repeating the same TS/sentry 6.x crossover search with TS 3.8 and 3.9 reveals a total of 5 projects), it does have the known side effect of replacing export statements which, after compilation, currently look likeexports.SdkInfo = types_1.SdkInfo;with ones that look likeObject.defineProperty(exports, "SdkInfo", { enumerable: true, get: function () { return types_1.SdkInfo; } });. (For those of you following along at home, that's a 3x character increase.) Though we might be able to engineer around this in order to avoid the inevitable substantial bundle size hit, attempting to do so is only worth it if there are features from 4.x that we desperately need. Given that we've agreed that right now there aren't, we'll stick with 3.8.3.