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

Ability to discern between types directive and x-TypeScript-types #133

Open
dsherret opened this issue Feb 9, 2022 · 2 comments
Open

Ability to discern between types directive and x-TypeScript-types #133

dsherret opened this issue Feb 9, 2022 · 2 comments
Assignees
Labels
enhancement New feature or request

Comments

@dsherret
Copy link
Member

dsherret commented Feb 9, 2022

For deno vendor, it might be nice to have a way to represent /// <reference types="..." /> and x-TypeScript-types separately in the graph while still providing a common method for the maybe_types_dependency.

This would also help us remove Range from being necessary for x-typescript-types as the types directive could have a range, but the x-typescript-types doesn't need one:

deno_graph/src/graph.rs

Lines 1331 to 1340 in f8adbe4

if let Some(types_header) = headers.get("x-typescript-types") {
let range = Range {
specifier: module.specifier.clone(),
start: Position::zeroed(),
end: Position::zeroed(),
};
let resolved_dependency = resolve(types_header, &range, maybe_resolver);
module.maybe_types_dependency =
Some((types_header.clone(), resolved_dependency));
}

Alternatively, I can just do a check for the start and end position all being zero, but that seems a little hacky.

@kitsonk kitsonk added the enhancement New feature or request label Feb 9, 2022
@kitsonk
Copy link
Contributor

kitsonk commented Feb 9, 2022

I don't think they should specifically be seperate, as they represent the same functional concept, but should contain an optional range instead of a 0 range.

@dsherret dsherret changed the title Store types directive and x-TypeScript-types separately in the graph Ability to discern between types directive and x-TypeScript-types Feb 10, 2022
@dsherret
Copy link
Member Author

Sure. I updated the title.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants