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

Add template variable ${configDir} for substitution of config files directory path #58042

Merged
merged 4 commits into from
Apr 16, 2024

Conversation

sheetalkamat
Copy link
Member

@sheetalkamat sheetalkamat commented Apr 2, 2024

  • ${configDir} on any paths is replced with config files directory instead of making it relative to file its declared in.
  • This is allowed on all the isFilePath options, paths, files, include and exclude
  • Not allowed to pass from command line

7b1a848 is just tests change
9fcd0d6 actual change

Fixes #57485

TODO:
Actual string to use for replacement from design meeting:

  • configLocation
  • configFolder
  • configDirectory
  • projectLocation
  • projectFolder
  • projectDirectory

Copy link
Member

@weswigham weswigham left a comment

Choose a reason for hiding this comment

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

Does a --showConfig on a config with paths like these show the input ${configDir} or the resolved disk paths?

src/compiler/commandLineParser.ts Show resolved Hide resolved
Copy link
Member

@weswigham weswigham left a comment

Choose a reason for hiding this comment

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

...Do we need to allow escaping $ so you can actually use a $ in a path, rather than as one of these templates? Like if I (somehow - the character is forbidden on NTFS) had an actual directory named ${configDir} - how do you escape from the pattern back to a literal interpretation?

@sheetalkamat
Copy link
Member Author

Does a --showConfig on a config with paths like these show the input ${configDir} or the resolved disk paths?

https://github.com/microsoft/TypeScript/pull/58042/files#diff-dfa255ce269b3c80a416b7c133db82a20292076b8c53370637b1aeaff7b8e7f3R110 shows resolved path and i think its useful to show it like that.

@sheetalkamat
Copy link
Member Author

...Do we need to allow escaping $ so you can actually use a $ in a path, rather than as one of these templates? Like if I (somehow - the character is forbidden on NTFS) had an actual directory named ${configDir} - how do you escape from the pattern back to a literal interpretation?

Not sure. i think we should keep this simple as otherwise we need to ensure all the paths are normalized to contain escaped $ we can choose other template pattern as well its a TODO but we shouldnt have to do escaping

@@ -1034,6 +1037,7 @@ const commandOptionsWithoutBuild: CommandLineOption[] = [
name: "paths",
type: "object",
affectsModuleResolution: true,
allowConfigDirTemplateSubstitution: true,
Copy link
Member

Choose a reason for hiding this comment

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

Long term, I do wonder if we'll need some other flag here given we'll introduce more substitutions, but thankfully this is internal!

@sheetalkamat sheetalkamat merged commit cbae6cf into main Apr 16, 2024
25 checks passed
@sheetalkamat sheetalkamat deleted the configDir branch April 16, 2024 23:25
Boshen added a commit to oxc-project/oxc-resolver that referenced this pull request Apr 23, 2024
…mpilerOptions.paths

closes #129

NOTE: All tests cases are just a head replacement of `${configDir}`, so we are constrained as such.

Reference: microsoft/TypeScript#58042
Boshen added a commit to oxc-project/oxc-resolver that referenced this pull request Apr 23, 2024
…mpilerOptions.paths

closes #129

NOTE: All tests cases are just a head replacement of `${configDir}`, so we are constrained as such.

Reference: microsoft/TypeScript#58042
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Author: Team For Uncommitted Bug PR for untriaged, rejected, closed or missing bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add template variable ${configDir} (name to be determined) for file path substitution
5 participants