Skip to content
This repository has been archived by the owner on May 22, 2024. It is now read-only.

fix: infer TS module format from package.json #1580

Merged
merged 3 commits into from
Oct 2, 2023

Conversation

eduardoboucas
Copy link
Member

Summary

Currently we're only looking at tsconfig.json to infer the module type. We need to also look at package.json as a fallback.

@eduardoboucas eduardoboucas requested a review from a team as a code owner October 2, 2023 15:14
@github-actions
Copy link
Contributor

github-actions bot commented Oct 2, 2023

⏱ Benchmark results

Comparing with 7aeed4d

largeDepsEsbuild: 3.2s

⬆️ 7.19% increase vs. 7aeed4d

^                           3.7s                                                                          
│                           ┌──┐                                                                          
│                           |  |                                                                    3.2s  
│                   3.1s    |  |            3.1s                                                    ┌──┐  
│                   ┌──┐    |  |            ┌──┐                                             3s     |▒▒|  
│ ──────────────────┼──┼────┼──┼────────────┼──┼────────────────────────────2.7s────────────┌──┐────|▒▒|──
│   2.6s            |  |    |  |    2.6s    |  |    2.6s    2.5s    2.5s    ┌──┐    2.6s    |  |    |▒▒|  
│   ┌──┐    2.4s    |  |    |  |    ┌──┐    |  |    ┌──┐    ┌──┐    ┌──┐    |  |    ┌──┐    |  |    |▒▒|  
│   |  |    ┌──┐    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
└───┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴──>
    T-12    T-11    T-10    T-9     T-8     T-7     T-6     T-5     T-4     T-3     T-2     T-1      T    
Legend

largeDepsNft: 9.3s

⬇️ 0.92% decrease vs. 7aeed4d

^                          11.7s                                                                          
│                           ┌──┐                                                                          
│                           |  |                                                                          
│                   9.8s    |  |            9.7s                                                          
│                   ┌──┐    |  |            ┌──┐                                            9.4s    9.3s  
│                   |  |    |  |            |  |                                            ┌──┐    ┌──┐  
│ ──────────────────┼──┼────┼──┼────8.2s────┼──┼────────────────────────────8.2s────8.1s────┼──┼────|▒▒|──
│   7.8s    7.5s    |  |    |  |    ┌──┐    |  |    7.9s    7.5s    7.6s    ┌──┐    ┌──┐    |  |    |▒▒|  
│   ┌──┐    ┌──┐    |  |    |  |    |  |    |  |    ┌──┐    ┌──┐    ┌──┐    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
└───┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴──>
    T-12    T-11    T-10    T-9     T-8     T-7     T-6     T-5     T-4     T-3     T-2     T-1      T    
Legend

largeDepsZisi: 18.3s

⬆️ 1.77% increase vs. 7aeed4d

^                          22.3s                                                                          
│                           ┌──┐                                                                          
│                           |  |                                                                          
│                  19.3s    |  |                                                                          
│                   ┌──┐    |  |           18.3s                                            18s    18.3s  
│                   |  |    |  |            ┌──┐                                            ┌──┐    ┌──┐  
│ ──────────────────┼──┼────┼──┼───15.4s────┼──┼───15.2s────────────────────16s────15.3s────┼──┼────|▒▒|──
│  14.6s   14.9s    |  |    |  |    ┌──┐    |  |    ┌──┐   14.4s   14.9s    ┌──┐    ┌──┐    |  |    |▒▒|  
│   ┌──┐    ┌──┐    |  |    |  |    |  |    |  |    |  |    ┌──┐    ┌──┐    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
└───┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴──>
    T-12    T-11    T-10    T-9     T-8     T-7     T-6     T-5     T-4     T-3     T-2     T-1      T    
Legend

lukasholzer
lukasholzer previously approved these changes Oct 2, 2023
Copy link
Contributor

@lukasholzer lukasholzer left a comment

Choose a reason for hiding this comment

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

Looks good to me, that's what I meant in Slack

Comment on lines 89 to 90
// Returns the module format that should be used when transpiling a TypeScript
// file.
Copy link
Contributor

Choose a reason for hiding this comment

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

NIT: comments for functions/methods or variables should always be JSDoc as the IntelliSense can pick them up. A regular code block comment should be just used inside functions as it is not part of the AST

Suggested change
// Returns the module format that should be used when transpiling a TypeScript
// file.
/**
* Returns the module format that should be used when transpiling a TypeScript
* file.
*/

Copy link
Member Author

Choose a reason for hiding this comment

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

Done in 99f27ba.

Comment on lines 9 to 11
// Looks for a `tsconfig.json` file on a given path and, if one exists, returns
// the module format inferred from the `module` property. If no file is found
// or if no `module` property is defined, the function returns `undefined`.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// Looks for a `tsconfig.json` file on a given path and, if one exists, returns
// the module format inferred from the `module` property. If no file is found
// or if no `module` property is defined, the function returns `undefined`.
/**
* Looks for a `tsconfig.json` file on a given path and, if one exists, returns
* the module format inferred from the `module` property. If no file is found
* or if no `module` property is defined, the function returns `undefined`.
*/

Same here

Copy link
Member Author

Choose a reason for hiding this comment

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

Done in 99f27ba.

@eduardoboucas eduardoboucas merged commit 028da38 into main Oct 2, 2023
@eduardoboucas eduardoboucas deleted the fix/ts-module-format branch October 2, 2023 16:10
Skn0tt pushed a commit to netlify/build that referenced this pull request May 21, 2024
…hip-it#1580)

* fix: infer TS module format from `package.json`

* chore: typo

* chore: update comments
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants