This repository was archived by the owner on Jun 11, 2020. It is now read-only.
Non-relative reference in typesVersions parent#781
Open
jablko wants to merge 1 commit intomicrosoft:masterfrom
Open
Non-relative reference in typesVersions parent#781jablko wants to merge 1 commit intomicrosoft:masterfrom
jablko wants to merge 1 commit intomicrosoft:masterfrom
Conversation
sandersn
suggested changes
Jun 5, 2020
Member
There was a problem hiding this comment.
This looks like a good idea, and I think the code is correct, but can you add a test? Additionally, the repo is now at microsoft/DefinitelyTyped-tools/packages/definitions-parser
In the DefinitelyTyped-tools repo, the tests are in DefinitelyTyped-tools/packages/definitions-parser/test/module-info.test.ts. I think the best approach is to test allReferencedFiles same as the other tests, then add a project that uses typesVersions to createMockDT in definitions-parser/src/mocks.ts.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Scenario 1
Currently, if you
import * as promises from "fs/promises"intypes/fs/ts3.2/index.d.ts(non-relative self-reference in a typesVersions directory),baseDirectoryists3.2subDirectoryis.convertToRelativeReference()returns./promises(thesubDirectory === "."case), which would betypes/fs/ts3.2/promises.d.ts✔️types-publisher/src/lib/module-info.ts
Lines 257 to 261 in 54bd71c
Scenario 2
However files in typesVersions directories are allowed to reference files in the parent directory, so if you
import * as promises from "fs/promises"intypes/fs/index.d.tsand/// <reference path="../index.d.ts" />intypes/fs/ts3.2/index.d.ts(non-relative self-reference in a typesVersions parent directory),baseDirectoryis stillts3.2subDirectoryis..convertToRelativeReference()returns../promises(thesubDirectory.split("/").lengthcase), which would betypes/promises.d.tsor something ❌That triggers
types-publisher/src/lib/module-info.ts
Lines 237 to 251 in 54bd71c
Proposed Change
If we're in a typesVersions parent directory (
baseDirectory && subDirectory.startsWith("..")) this change adds thebaseDirectoryto the relative reference, so in scenario 2,convertToRelativeReference()returns./ts3.2/promises, which would betypes/fs/ts3.2/promises.d.ts, the same as in scenario 1 ✔️