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

Support multiple ranges for an import in the graph #132

Closed
kitsonk opened this issue Feb 8, 2022 · 3 comments
Closed

Support multiple ranges for an import in the graph #132

kitsonk opened this issue Feb 8, 2022 · 3 comments
Assignees
Labels
enhancement New feature or request

Comments

@kitsonk
Copy link
Contributor

kitsonk commented Feb 8, 2022

Currently the graph cannot represent multiple locations an import specifier might be imported in a file. For example, an import can be like this:

import * as a from "https://example.com/a";
import { default as b } from "https://example.com/a";

In addition, in theory, a module can be statically and dynamically imported:

import * as a from "https://example.com/a";
const b = await import("https://example.com/a";

Or in TypeScript the same specifier can be provided for a type only import:

import { a } from "https://example.com/a";
import type { A } from "https://example.com/a";

Currently we only store the last range for the specifier in the graph. This makes it difficult when trying to rewrite specifiers (like in dnt or deno vendor) as well as means that diagnostics are only issued for the "last" import location, not all import locations for a given specifier.

We should enhance the graph to be able to store this information, which is likely going to be a breaking change, but one that increases the usability of the graph.

@kitsonk kitsonk added the enhancement New feature or request label Feb 8, 2022
@nayeemrmn
Copy link
Collaborator

This is a must for proper import assertion handling. Import assertion errors should be stored under each of these, not occupy module slots.

@dsherret
Copy link
Member

dsherret commented Feb 9, 2022

I've run into needing this for deno vendor again while working on the import map only solution. It also needs to know the text for the // @deno-types="..." and /// <reference types="..." /> directives so that it can tell if it needs to map those.

@nayeemrmn
Copy link
Collaborator

Closed in #251.

@dsherret dsherret closed this as completed Sep 6, 2023
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

Successfully merging a pull request may close this issue.

3 participants