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.

Sign up for GitHub

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

chore: add deno to the providers in defaultProviders comment #369

Closed
wants to merge 3 commits into from
+1 −1
Closed

chore: add deno to the providers in defaultProviders comment #369

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f828d51
chore: add deno to the providers in defaultProviders comment
JayaKrishnaNamburu Jul 12, 2024
2b643df
Merge branch 'main' into fix-provider-comment
JayaKrishnaNamburu Jul 18, 2024
f9e58b9
update the provider to denoland in comment
JayaKrishnaNamburu Jul 18, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/generator.ts
View file
Original file line number Diff line number Diff line change
@@ -101,7 +101,7 @@ export interface GeneratorOptions {
/**
* The provider to use for top-level (i.e. root package) installs if there's no context in the inputMap. This can be used to set the provider for a new import map. To use a specific provider for an install, rather than relying on context, register an override using the 'providers' option.
*
* Supports: 'jspm.io' | 'jspm.io#system' | 'nodemodules' | 'skypack' | 'jsdelivr' | 'unpkg' | 'esm.sh';
* Supports: 'jspm.io' | 'denoland' | 'nodemodules' | 'skypack' | 'jsdelivr' | 'unpkg' | 'esm.sh';
Copy link
Member

Choose a reason for hiding this comment

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

Should this not be defaultRegistry: 'denoland'?

Copy link
Member Author

Choose a reason for hiding this comment

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

I changed it for defaultProvider not the registry 👍

Copy link
Member

@guybedford guybedford Jul 18, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I don't think this works actually, since denoland is not the provider. The provider is deno and the registry is either deno or denoland. I think the mistake in the discussion was mixing up the registry and provider terms, I meant to say that it's the denoland registry not the denoland provider that should be used.

I believe the denoland provider is an error, since it is not registered here - https://github.com/jspm/generator/blob/main/src/providers/index.ts#L55.

*
* Providers are responsible for resolution from abstract package names and version ranges to exact URL locations.
*