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.

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

feat(TS_CONFIG_PATH): TS_CONFIG_PATH environment variable for forcing tsconfig loading #64

Merged
merged 5 commits into from
May 27, 2020
Merged

feat(TS_CONFIG_PATH): TS_CONFIG_PATH environment variable for forcing tsconfig loading #64

merged 5 commits into from
May 27, 2020

Conversation

EntraptaJ
Copy link
Member

@EntraptaJ EntraptaJ commented May 17, 2020 •
edited
Loading

This PR adds the option to provide TS_CONFIG_PATH environment variable to force the path to the TSConfig file to use for transpiling and paths.

@EntraptaJ EntraptaJ added the enhancement New feature or request label May 17, 2020
@EntraptaJ EntraptaJ linked an issue May 17, 2020 that may be closed by this pull request
TS_PROJECT Environment Variable Support #61
Closed
@EntraptaJ
Copy link
Member Author

@yoursunny This is one way to implement this, I could also have it be the path of the actual config/project file.

@EntraptaJ
Copy link
Member Author

I'm working on refactoring my testing infrastructure so I can add a proper test, it's actually how the Type-Checking thing got started. I need a test that ensures that a tsconfig.json file is being loaded, but since my testing infrastructure and TS-ESNode itself doesn't actually do typechecking I'm having some tiny issues with that.

@yoursunny
Copy link
Contributor

I could also have it be the path of the actual config/project file.

I'd prefer being able to specify full path to tsconfig.json, such as TS_PROJECT=$(pwd)/mk/tsconfig-run.json.
This allows the flexibility that the file does not have to be named tsconfig.json.

The simplest implementation is:

export function getTSConfig(modulePath: string): CompilerOptions {
  const tsConfigPath = process.env.TS_CONFIG_PATH ?? ts.findConfigFile(modulePath, ts.sys.fileExists);

@yoursunny
Copy link
Contributor

I tested this change along with #80 in NDNts repository, and they appear to work well without using my custom hacks.

export TS_CONFIG_PATH=$ROOTDIR/mk/tsconfig-literate.json
node --loader @k-foss/ts-esnode --experimental-specifier-resolution=node literate-temp.ts

You may want to rename this PR to reflect the changed environment variable name.

Currently this change does not recognize TSConfig extends, so I have to copy compilerOptions into the final TSConfig, which is a minor annoyance.
Maybe ts.parseJsonConfigFileContent should be used instead of ts.readConfigFile.

@EntraptaJ EntraptaJ changed the title feat(TS_PROJECT): TS_PROJECT environment variable for search feat(TS_CONFIG_PATH): TS_CONFIG_PATH environment variable for forcing tsconfig loading May 27, 2020
@EntraptaJ
Copy link
Member Author

I tested this change along with #80 in NDNts repository, and they appear to work well without using my custom hacks.

export TS_CONFIG_PATH=$ROOTDIR/mk/tsconfig-literate.json
node --loader @k-foss/ts-esnode --experimental-specifier-resolution=node literate-temp.ts

You may want to rename this PR to reflect the changed environment variable name.

Currently this change does not recognize TSConfig extends, so I have to copy compilerOptions into the final TSConfig, which is a minor annoyance.
Maybe ts.parseJsonConfigFileContent should be used instead of ts.readConfigFile.

I've renamed the PR and fixed the description. And... I got "extends" working. The current fs.readFile is only temp until I get my WIP cache system I'm trying to work towards in place.

@EntraptaJ EntraptaJ marked this pull request as ready for review May 27, 2020 16:42
EntraptaJ
EntraptaJ commented May 27, 2020
View reviewed changes
src/Utils.ts
Comment on lines +25 to 32
const jsonText = await fs.readFile(tsConfigPath);
const result = ts.parseJsonText(tsConfigPath, jsonText.toString());

tsConfigCache = ts.convertCompilerOptionsFromJson(
tsConfigFile.compilerOptions,
pathDirname(tsConfigPath),
tsConfigCache = ts.parseJsonSourceFileConfigFileContent(
result,
ts.sys,
dirname(tsConfigPath),
).options;
Copy link
Member Author

Choose a reason for hiding this comment

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

This will likely have a small perf impact until I setup proper caching. Need to finish moving this repo over to using @K-FOSS/TS-ESTests and have proper time differences on each PR/commit. Speed is my main priority with TS-ESNode.

yoursunny
yoursunny approved these changes May 27, 2020
View reviewed changes
Copy link
Contributor

@yoursunny yoursunny left a comment

Choose a reason for hiding this comment

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

I tested this (after git merge master) in NDNts codebase and it works well. "extends" is working too.

@EntraptaJ EntraptaJ merged commit 5885d2e into master May 27, 2020
@EntraptaJ
Copy link
Member Author

@yoursunny Release should be live in a few minutes.

github-actions bot pushed a commit that referenced this pull request May 27, 2020
@KJDev-Bot
chore(release): 1.6.0 [skip ci]
2629785 
# [1.6.0](v1.5.1...v1.6.0) (2020-05-27)

### Features

* **TS_CONFIG_PATH:** `TS_CONFIG_PATH` environment variable for forcing tsconfig loading ([#64](#64)) ([5885d2e](5885d2e))
@KJDev-Bot
Copy link
Contributor

🎉 This PR is included in version 1.6.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@EntraptaJ EntraptaJ deleted the feat-ts-project branch May 31, 2020 01:17
@yoursunny yoursunny mentioned this pull request Oct 29, 2021
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@yoursunny yoursunny yoursunny approved these changes

Assignees
No one assigned
Labels
enhancement New feature or request released
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

TS_PROJECT Environment Variable Support
3 participants
@EntraptaJ @yoursunny @KJDev-Bot