Skip to content
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

Resolve extended TypeScript configuration files #2240

Merged
merged 1 commit into from
Oct 11, 2021

Conversation

mrmckeb
Copy link
Contributor

@mrmckeb mrmckeb commented Sep 28, 2021

This PR resolves #1908.

In a comparison I ran locally, the output is:

// New method - includes extended configuration
{
  options: {
    esModuleInterop: true,
    forceConsistentCasingInFileNames: true,
    noFallthroughCasesInSwitch: true,
    skipLibCheck: true,
    strict: true,
    baseUrl: '[redacted]/tsconfig.json',
    allowJs: true,
    jsx: 1,
    target: 99,
    module: 99,
    lib: [ 'lib.dom.d.ts', 'lib.esnext.d.ts' ],
    noEmit: true,
    moduleResolution: 2,
    resolveJsonModule: true,
    isolatedModules: true,
    incremental: true,
    paths: { '~/*': [ './*' ], '@*': [ '../shared/*' ] },
    pathsBasePath: '[redacted]/tsconfig.json',
    configFilePath: undefined
  }
  // ...
}

// Old method - only references the config to extend.
{
  extends: '[extended-config-name]',
  compilerOptions: {
    baseUrl: '.',
    allowJs: true,
    jsx: 'preserve',
    target: 'ESNext',
    module: 'ESNext',
    lib: [ 'DOM', 'ESNext' ],
    noEmit: true,
    moduleResolution: 'node',
    resolveJsonModule: true,
    isolatedModules: true,
    incremental: true,
    paths: { '~/*': [Array], '@*': [Array] }
  },
  // ...
}

For reference, the shared config has:

{
  "compilerOptions": {
    "esModuleInterop": true,
    "forceConsistentCasingInFileNames": true,
    "noFallthroughCasesInSwitch": true,
    "skipLibCheck": true,
    "strict": true
  }
}

@codecov
Copy link

codecov bot commented Sep 28, 2021

Codecov Report

Merging #2240 (e08bf2a) into main (dd81424) will decrease coverage by 0.59%.
The diff coverage is n/a.

❗ Current head e08bf2a differs from pull request most recent head 9a744f7. Consider uploading reports for the commit 9a744f7 to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##             main    #2240      +/-   ##
==========================================
- Coverage   95.23%   94.63%   -0.60%     
==========================================
  Files          65       65              
  Lines        2686     2686              
  Branches      888      888              
==========================================
- Hits         2558     2542      -16     
- Misses        128      144      +16     
Impacted Files Coverage Δ
src/ExportMap.js 92.85% <ø> (ø)
src/rules/no-unresolved.js 83.33% <0.00%> (-16.67%) ⬇️
utils/resolve.js 88.13% <0.00%> (-11.87%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update dd81424...9a744f7. Read the comment docs.

src/ExportMap.js Outdated Show resolved Hide resolved
tests/src/ExportMap.js Outdated Show resolved Hide resolved
src/ExportMap.js Outdated Show resolved Hide resolved
src/ExportMap.js Outdated Show resolved Hide resolved
src/ExportMap.js Outdated Show resolved Hide resolved
src/ExportMap.js Show resolved Hide resolved
@mrmckeb
Copy link
Contributor Author

mrmckeb commented Sep 28, 2021

Thanks for the feedback @ljharb. Let me know if any other changes are needed and I can make them tomorrow :)

Copy link
Member

@ljharb ljharb left a comment

Choose a reason for hiding this comment

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

Looks great!

@ljharb ljharb closed this Oct 5, 2021
@ljharb ljharb reopened this Oct 5, 2021
@ljharb
Copy link
Member

ljharb commented Oct 5, 2021

Looks like this PR may have broken the WSL windows builds.

@mrmckeb
Copy link
Contributor Author

mrmckeb commented Oct 6, 2021

The errors I'm seeing on Appveyor are:

## Running `apt-get update` for you...
+ apt-get update
Hit:1 http://security.ubuntu.com/ubuntu focal-security InRelease
Ign:2 https://deb.nodesource.com/node_16.x focal InRelease
Err:3 https://deb.nodesource.com/node_16.x focal Release
  Certificate verification failed: The certificate is NOT trusted. The certificate chain uses expired certificate.  Could not handshake: Error in the certificate verification. [IP: 23.220.206.33 443]
Hit:4 http://archive.ubuntu.com/ubuntu focal InRelease
Hit:5 http://archive.ubuntu.com/ubuntu focal-updates InRelease
Hit:6 http://archive.ubuntu.com/ubuntu focal-backports InRelease
Reading package lists...
wsl : E: The repository 'https://deb.nodesource.com/node_16.x focal Release' does not have a Release file.
At line:1 char:1
+ wsl curl -sL 'https://deb.nodesource.com/setup_${nodejs_version}.x' ` ...
+ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    + CategoryInfo          : NotSpecified: (E: The reposito...a Release file.:String) [], RemoteException
    + FullyQualifiedErrorId : NativeCommandError
 
Error executing command, exiting
Command executed with exception: E: The repository 'https://deb.nodesource.com/node_16.x focal Release' does not have a Release file.

Which seems like it could be something else? Windows 11 has kind of been released in the last day or so, could be related?

@ljharb
Copy link
Member

ljharb commented Oct 6, 2021

oh hm - "error in certificate verification" might mean it's related to the LetsEncrypt CA expiration last week?

@ljharb
Copy link
Member

ljharb commented Oct 10, 2021

@mrmckeb
Copy link
Contributor Author

mrmckeb commented Oct 10, 2021

Thanks for the updates @ljharb!

@ljharb
Copy link
Member

ljharb commented Oct 11, 2021

Seems like appveyor fixed itself.

@ljharb ljharb merged commit 9a744f7 into import-js:main Oct 11, 2021
@mrmckeb mrmckeb deleted the fix-tsconfig-resolution branch October 11, 2021 06:12
@mrmckeb
Copy link
Contributor Author

mrmckeb commented Oct 11, 2021

Thank you!

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

Successfully merging this pull request may close these issues.

[import/default] typescript - esModuleInterop value not read correctly
3 participants