Skip to content

Conversation

@sheetalkamat
Copy link
Member

Fixes #35734

Copy link
Member

@sandersn sandersn left a comment

Choose a reason for hiding this comment

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

Code looks OK, but do we lose any ability to detect or create dynamic file names since real files can use carat now?

@sheetalkamat
Copy link
Member Author

do we lose any ability to detect or create dynamic file names since real files can use carat now?

We already have tests for all the scenarios that use dynamic file names and they work, (I think work to retain projectRootPath for open files has rendered this not needed any more)

@sheetalkamat
Copy link
Member Author

@typescript-bot pack this

@typescript-bot
Copy link
Collaborator

typescript-bot commented Jan 10, 2020

Heya @sheetalkamat, I've started to run the tarball bundle task on this PR at 4858f4b. You can monitor the build here. It should now contribute to this PR's status checks.

@sheetalkamat
Copy link
Member Author

@uniqueiniquity @jessetrinity @amcasey can you try our the drop here to see the debugging scenarios are working ok.
I tried creating bunch of dynamic files in VS but I might not have covered all scenarios. (file is created dynamic when debugging in ie and clicking files from solution explorer and script documents list if there is not matching file found on disk.. eg you compile ts with --inlineSources and then delete the ts files)

@uniqueiniquity
Copy link
Contributor

@sheetalkamat sure thing - I actually just built it locally a few moments ago.

@typescript-bot
Copy link
Collaborator

typescript-bot commented Jan 10, 2020

Hey @sheetalkamat, I've packed this into an installable tgz. You can install it for testing by referencing it in your package.json like so:

{
    "devDependencies": {
        "typescript": "https://typescript.visualstudio.com/cf7ac146-d525-443c-b23c-0d58337efebc/_apis/build/builds/59993/artifacts?artifactName=tgz&fileId=8AAE0DD5296BDA99EDEF45782C5550B9CDEE791E097459DD7D16A0FBB90E9F6802&fileName=/typescript-3.8.0-insiders.20200110.tgz"
    }
}

and then running npm install.


There is also a playground for this build.

@uniqueiniquity
Copy link
Contributor

@sheetalkamat Looks good to me. Tried naming normal files with a leading ^, tried using dynamic files, and tried using dynamic files that start with a leading ^. All seems fine.

@sheetalkamat sheetalkamat merged commit 00b21ef into master Jan 10, 2020
@sheetalkamat sheetalkamat deleted the dynamicFileName branch January 10, 2020 18:57
Kingwl pushed a commit to Kingwl/TypeScript that referenced this pull request Mar 4, 2020
@microsoft microsoft locked as resolved and limited conversation to collaborators Oct 21, 2025
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.

Debug Failure. False expression: in openExternalProjects in getOrCreateScriptInfoWorker

5 participants