Skip to content

Bug: Inconsistent TSMappedType AST shape #11055

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
Open
4 tasks done
JLHwung opened this issue Apr 9, 2025 · 4 comments · May be fixed by #11086
Open
4 tasks done

Bug: Inconsistent TSMappedType AST shape #11055

JLHwung opened this issue Apr 9, 2025 · 4 comments · May be fixed by #11086
Labels
accepting prs Go ahead, send a pull request that resolves this issue AST PRs and Issues about the AST structure bug Something isn't working package: typescript-estree Issues related to @typescript-eslint/typescript-estree

Comments

@JLHwung
Copy link

JLHwung commented Apr 9, 2025

Before You File a Bug Report Please Confirm You Have Done The Following...

  • I have tried restarting my IDE and the issue persists.
  • I have updated to the latest version of the packages.
  • I have searched for related issues and found none that matched my issue.
  • I have read the FAQ and my problem is not listed.

Relevant Package

ast-spec

Playground Link

https://typescript-eslint.io/play/#ts=5.8.3&showAST=es&fileType=.ts&code=G4QwTgBCBcEN4QNoGsIEsB2UME8C6sIuEAvgFBmiQBGsCK6WR%2BA-IceZeBAMZ0RgApiAAmAewwAbHElSZs%2BdjM5UII-gGohoidNmMFeNgtIVVg-gFpt4qTIbzmBEySA&eslintrc=N4KABGBEBOCuA2BTAzpAXGYBfEWg&tsconfig=N4KABGBEDGD2C2AHAlgGwKYCcDyiAuysAdgM6QBcYoEEkJemy0eAcgK6qoDCAFutAGsylBm3TgwAXxCSgA&tokens=true

Repro Code

var a: { [k in any]: any }

var b: { [k in any]?: any }

var c: { readonly [k in any]: any }

var d: { +readonly [k in any]?: any }

var e: { -readonly [k in any]: any }

ESLint Config



tsconfig




Expected Result


The 1st TSMappedType should have properties { optional: false, readonly: undefined }


The 2nd TSMappedType should have property { readonly: undefined }


The 3rd TSMappedType should have property { optional: false }


The 5th TSMappedType should have property { optional: false }


Actual Result


The aforementioned TSMappedTypes are missing such properties, resulted to inconsistent AST shapes.


Additional Info


No response


Versions







package
version






@typescript-eslint/typescript-estree
8.29.1





      		
    

      
  





        



            
            

              
            

        

      


      
    








      

    



      

  
            

    

      
    

    


      

          @JLHwung
JLHwung




            added
      

  bug

  Something isn't working

      

  triage

  Waiting for team members to take a look

  labels


      Apr 9, 2025

    

  








      

  
      



  

  @JoshuaKGoldberg





  


    

      

  

    

        
    
    

  


      
          
  
      
            Copy link

  

      
    

  


  

      

  
  Member


      

  


  

    

      

      
            JoshuaKGoldberg
  

      

      

      commented


        Apr 9, 2025


      
  •

  

    
      

        
          edited
          
        
        
      

    
    
      
        
      Loading

    
  



    


  





      


        


  
    

      
          
#5020 - is there a reason the AST should have the falsy properties defined?


Edit: I think I'd meant to ask "shouldn't", heh

      		
    

  





        


            

            
            

              
            

        

      


      

            
  

    
  
    
    

  

  


    












      

  
            

    

      
    

    


      

          @JoshuaKGoldberg
JoshuaKGoldberg




            added
      

  awaiting response

  Issues waiting for a reply from the OP or another party

    and removed
      

  triage

  Waiting for team members to take a look

  labels


      Apr 9, 2025

    

  








      

  
      



  

  @JLHwung





  


    

      

  

    

        
    
    

  


      
          
  
      
            Copy link

  

      
    

  


  

      



      

  Author


  


  

    

      

      
            JLHwung
  

      

      

      commented


        Apr 9, 2025


      
  •

  

    
      

        
          edited
          
        
        
      

    
    
      
        
      Loading

    
  



    


  





      


        


  
    

      
          
I believe this issue is exactly part of #5020:


For example, the second one var b: { [k in any]?: any } is represented by a TSMappedType with an optional: true property.


while the first one var a: { [k in any]: any } is represented by a TSMappedType without the optional property. So that means the optional property is optional as of TSMappedType. The conclusion of #5020 is that




Having a standard shape should help V8 optimise things at the (allbeit small) memory impact of additional "useless" values.




So we should always define the optional property, ditto for the readonly property.

      		
    

  





        


            

            
            

              
            

        

      


      

            
  

    
  
    
    

  

  


    












      

  
            

    

      
    

    


      

          @bradzacher
bradzacher




            added
      

  package: typescript-estree

  Issues related to @typescript-eslint/typescript-estree

      

  AST

  PRs and Issues about the AST structure

      

  accepting prs

  Go ahead, send a pull request that resolves this issue

    and removed
      

  awaiting response

  Issues waiting for a reply from the OP or another party

  labels


      Apr 10, 2025

    

  








      

  
      



  

  @bradzacher





  


    

      

  

    

        
    
    

  


      
          
  
      
            Copy link

  

      
    

  


  

      

  
  Member


      

  


  

    

  





      


        


  
    

      
          
This is a bug in the translation logic yeah -- it should always define the boolean flags as optional: false and readonly: false -- we should never omit them.

      		
    

  





        


            

            
            

              
            

        

      


      

            
  

    
  
    
    

  

  


    












      

  
      



  

  @JLHwung





  


    

      

  

    

        
    
    

  


      
          
  
      
            Copy link

  

      
    

  


  

      



      

  Author


  


  

    

      

      
            JLHwung
  

      

      

      commented


        Apr 10, 2025


      
    


  





      


        


  
    

      
          
Note that readonly is an enum of true, + and -. So previously I proposed undefined as the default value. I guess false does no harm here.

      		
    

  





        


            

            
            

              
            

        

      


      

            
  

    
  
    
    

  

  


    












      

  
          

  

    
  


  

      

    @dbarabashh
dbarabashh



    linked a pull request
    
      Apr 17, 2025
    

    that will
    close
    this issue





  


  

      
   Open

  





  
  

      
        
      
      

  
    
    
  


    3 tasks
  





  













  
  

    

      

  





  




  
  

              

  
    Sign up for free
    to join this conversation on GitHub.
    Already have an account?
    Sign in to comment


  



  






  
                  




    

  


      
  

    Assignees
  



        

    No one assigned







      


  


  

    Labels
  



    

      

    accepting prs

  Go ahead, send a pull request that resolves this issue
  

    AST

  PRs and Issues about the AST structure
  

    bug

  Something isn't working
  

    package: typescript-estree

  Issues related to @typescript-eslint/typescript-estree








      

  

    

        

    Projects
  


        




    None yet




  



      


  

    
  

    Milestone
  


      No milestone





    
      
          


  

    

      
        

          
    

    Development
  




            
  
Successfully merging a pull request may close this issue.





    


  
          
      fix(eslint-plugin): ensure consistent TSMappedType AST shape

    
      dbarabashh/typescript-eslint
    





      
    

  




      

    

  
      

  

    

      3 participants
    

    

        
          @JoshuaKGoldberg 
        
          @JLHwung 
        
          @bradzacher