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

feat(vendor): support using an existing import map #14836

Merged
merged 29 commits into from
Jun 14, 2022

Conversation

dsherret
Copy link
Member

@dsherret dsherret commented Jun 9, 2022

Take 2 (initial attempt was #14824).

  1. Keeps the import map in the ./vendor folder.
  2. Includes any necessary mappings from the original import map in the output.

Closes #13697

@dsherret dsherret marked this pull request as draft June 9, 2022 21:35
@dsherret dsherret marked this pull request as ready for review June 9, 2022 22:37
Copy link
Member

@lucacasonato lucacasonato left a comment

Choose a reason for hiding this comment

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

Tested this in three repositories that use import maps, and it works correctly in all three. Very nice!

Comment on lines 55 to 59
"\nTo use vendored modules, specify the `--import-map {}` flag when ",
r#"invoking deno subcommands or add an `"importMap": "<path_to_vendored_import_map>"` "#,
"entry to your deno.json file.",
),
raw_output_dir.join("import_map.json").display(),
Copy link
Member

Choose a reason for hiding this comment

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

Why is there an actual specifier for --import-map, but not for <path_to_vendored_import_map>?

Copy link
Member Author

@dsherret dsherret Jun 10, 2022

Choose a reason for hiding this comment

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

It's more complicated and I didn't think it was worth it. It would require checking where the config is and then doing different logic for whether the config existed or didn't exist. Plus, now it auto updates the config file if it exists.

@bartlomieju
Copy link
Member

@dsherret any chance we can land that for v1.23?

@dsherret dsherret added this to the 1.23 milestone Jun 14, 2022
@dsherret dsherret merged commit 443041c into denoland:main Jun 14, 2022
@dsherret dsherret deleted the deno_vendor_existing_import_map branch June 14, 2022 14:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

deno vendor does not work with existing import map with bare specifier mapping
3 participants