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

fix: make tsconfig-paths work with comments in tsconfig.json #588

Merged
merged 1 commit into from Sep 14, 2020
Merged

fix: make tsconfig-paths work with comments in tsconfig.json #588

merged 1 commit into from Sep 14, 2020

Conversation

RafaelKr
Copy link
Contributor

When you run tsc --init it creates a tsconfig.json which includes two types of comments:

  1. // single line
  2. /* multiline */

When using require() to load this tsconfig.json with comments it throws an error:
SyntaxError: /path/to/tsconfig.json: Unexpected token / in JSON at position 29

This is why tsconfig-paths uses it's own loadTsconfig function to parse that JSON file.
So we can simply use that instead of require()

tsconfig.json from "tsc --init"
{
  "compilerOptions": {
    /* Visit https://aka.ms/tsconfig.json to read more about this file */

    /* Basic Options */
    // "incremental": true,                   /* Enable incremental compilation */
    "target": "es5",                          /* Specify ECMAScript target version: 'ES3' (default), 'ES5', 'ES2015', 'ES2016', 'ES2017', 'ES2018', 'ES2019', 'ES2020', or 'ESNEXT'. */
    "module": "commonjs",                     /* Specify module code generation: 'none', 'commonjs', 'amd', 'system', 'umd', 'es2015', 'es2020', or 'ESNext'. */
    // "lib": [],                             /* Specify library files to be included in the compilation. */
    // "allowJs": true,                       /* Allow javascript files to be compiled. */
    // "checkJs": true,                       /* Report errors in .js files. */
    // "jsx": "preserve",                     /* Specify JSX code generation: 'preserve', 'react-native', or 'react'. */
    // "declaration": true,                   /* Generates corresponding '.d.ts' file. */
    // "declarationMap": true,                /* Generates a sourcemap for each corresponding '.d.ts' file. */
    // "sourceMap": true,                     /* Generates corresponding '.map' file. */
    // "outFile": "./",                       /* Concatenate and emit output to single file. */
    // "outDir": "./",                        /* Redirect output structure to the directory. */
    // "rootDir": "./",                       /* Specify the root directory of input files. Use to control the output directory structure with --outDir. */
    // "composite": true,                     /* Enable project compilation */
    // "tsBuildInfoFile": "./",               /* Specify file to store incremental compilation information */
    // "removeComments": true,                /* Do not emit comments to output. */
    // "noEmit": true,                        /* Do not emit outputs. */
    // "importHelpers": true,                 /* Import emit helpers from 'tslib'. */
    // "downlevelIteration": true,            /* Provide full support for iterables in 'for-of', spread, and destructuring when targeting 'ES5' or 'ES3'. */
    // "isolatedModules": true,               /* Transpile each file as a separate module (similar to 'ts.transpileModule'). */

    /* Strict Type-Checking Options */
    "strict": true,                           /* Enable all strict type-checking options. */
    // "noImplicitAny": true,                 /* Raise error on expressions and declarations with an implied 'any' type. */
    // "strictNullChecks": true,              /* Enable strict null checks. */
    // "strictFunctionTypes": true,           /* Enable strict checking of function types. */
    // "strictBindCallApply": true,           /* Enable strict 'bind', 'call', and 'apply' methods on functions. */
    // "strictPropertyInitialization": true,  /* Enable strict checking of property initialization in classes. */
    // "noImplicitThis": true,                /* Raise error on 'this' expressions with an implied 'any' type. */
    // "alwaysStrict": true,                  /* Parse in strict mode and emit "use strict" for each source file. */

    /* Additional Checks */
    // "noUnusedLocals": true,                /* Report errors on unused locals. */
    // "noUnusedParameters": true,            /* Report errors on unused parameters. */
    // "noImplicitReturns": true,             /* Report error when not all code paths in function return a value. */
    // "noFallthroughCasesInSwitch": true,    /* Report errors for fallthrough cases in switch statement. */

    /* Module Resolution Options */
    // "moduleResolution": "node",            /* Specify module resolution strategy: 'node' (Node.js) or 'classic' (TypeScript pre-1.6). */
    // "baseUrl": "./",                       /* Base directory to resolve non-absolute module names. */
    // "paths": {},                           /* A series of entries which re-map imports to lookup locations relative to the 'baseUrl'. */
    // "rootDirs": [],                        /* List of root folders whose combined content represents the structure of the project at runtime. */
    // "typeRoots": [],                       /* List of folders to include type definitions from. */
    // "types": [],                           /* Type declaration files to be included in compilation. */
    // "allowSyntheticDefaultImports": true,  /* Allow default imports from modules with no default export. This does not affect code emit, just typechecking. */
    "esModuleInterop": true,                  /* Enables emit interoperability between CommonJS and ES Modules via creation of namespace objects for all imports. Implies 'allowSyntheticDefaultImports'. */
    // "preserveSymlinks": true,              /* Do not resolve the real path of symlinks. */
    // "allowUmdGlobalAccess": true,          /* Allow accessing UMD globals from modules. */

    /* Source Map Options */
    // "sourceRoot": "",                      /* Specify the location where debugger should locate TypeScript files instead of source locations. */
    // "mapRoot": "",                         /* Specify the location where debugger should locate map files instead of generated locations. */
    // "inlineSourceMap": true,               /* Emit a single file with source maps instead of having a separate file. */
    // "inlineSources": true,                 /* Emit the source alongside the sourcemaps within a single file; requires '--inlineSourceMap' or '--sourceMap' to be set. */

    /* Experimental Options */
    // "experimentalDecorators": true,        /* Enables experimental support for ES7 decorators. */
    // "emitDecoratorMetadata": true,         /* Enables experimental support for emitting type metadata for decorators. */

    /* Advanced Options */
    "skipLibCheck": true,                     /* Skip type checking of declaration files. */
    "forceConsistentCasingInFileNames": true  /* Disallow inconsistently-cased references to the same file. */
  }
}

Maybe Related:
#150
#170
#291
vercel/vercel#2832

When you run `tsc --init` it creates a tsconfig.json which includes two types of comments:
1. // single line
2. /* multiline */

When using `require()` to load this tsconfig.json with comments it throws an error:
`SyntaxError: /path/to/tsconfig.json: Unexpected token / in JSON at position 29`

This is why tsconfig-paths uses it's own `loadTsconfig` function to parse that JSON file.
So we can simply use that instead of `require()`
@RafaelKr
Copy link
Contributor Author

RafaelKr commented Sep 12, 2020

I also tried to add a test, but I couldn't run the tests locally

$ npm run test         

> @vercel/ncc@0.24.0 test /home/rafael/coding/github/ncc
> node --expose-gc --max_old_space_size=3072 node_modules/.bin/jest

 FAIL  test/index.test.js
  ● Test suite failed to run

    Cannot find module '../' from 'test/index.test.js'

      2 | const { fork } = require("child_process");
      3 | const coverage = global.coverage;
    > 4 | const ncc = coverage ? require("../src/index") : require("../");
        |                                                  ^
      5 | 
      6 | for (const unitTest of fs.readdirSync(`${__dirname}/unit`)) {
      7 |   it(`should generate correct output for ${unitTest}`, async () => {

      at Resolver.resolveModule (node_modules/jest-resolve/build/index.js:307:11)
      at Object.<anonymous> (test/index.test.js:4:50)

 FAIL  test/watcher.test.js
  ● Test suite failed to run

    Cannot find module '../' from 'test/watcher.test.js'

      1 | const path = require('path');
    > 2 | const ncc = global.coverage ? require("../src/index") : require("../");
        |                                                         ^
      3 | const fs = require('fs');
      4 | 
      5 | // Based on the NodeWatchFileSystem class at:

      at Resolver.resolveModule (node_modules/jest-resolve/build/index.js:307:11)
      at Object.<anonymous> (test/watcher.test.js:2:57)

Test Suites: 2 failed, 2 total
Tests:       0 total
Snapshots:   0 total
Time:        0.781 s, estimated 232 s
Ran all test suites.
npm ERR! code ELIFECYCLE
npm ERR! errno 1
npm ERR! @vercel/ncc@0.24.0 test: `node --expose-gc --max_old_space_size=3072 node_modules/.bin/jest`
npm ERR! Exit status 1
npm ERR! 
npm ERR! Failed at the @vercel/ncc@0.24.0 test script.
npm ERR! This is probably not a problem with npm. There is likely additional logging output above.

npm ERR! A complete log of this run can be found in:
npm ERR!     /home/rafael/.npm/_logs/2020-09-12T22_33_44_374Z-debug.log

RafaelKr added a commit to RafaelKr/olive-issue-triage-bot that referenced this pull request Sep 12, 2020
Copy link
Member

@styfle styfle left a comment

Choose a reason for hiding this comment

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

Thanks!

@styfle styfle added the automerge Automatically merge PR once checks pass label Sep 14, 2020
@kodiakhq kodiakhq bot merged commit 987ca57 into vercel:master Sep 14, 2020
@RafaelKr RafaelKr deleted the fix/tsconfig-paths-with-tsconfig-comments branch September 14, 2020 13:43
@RafaelKr RafaelKr restored the fix/tsconfig-paths-with-tsconfig-comments branch September 14, 2020 13:43
@RafaelKr RafaelKr deleted the fix/tsconfig-paths-with-tsconfig-comments branch September 14, 2020 13:43
@RafaelKr
Copy link
Contributor Author

@styfle Thanks for merging! Do you know when this will be released?

@RafaelKr
Copy link
Contributor Author

Thanks for the quick release!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automerge Automatically merge PR once checks pass
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants