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

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

Closed
wants to merge 3 commits into from

Conversation

JayaKrishnaNamburu
Copy link
Member

Just a small comment change, mis-leading in editor suggestions.

@guybedford
Copy link
Member

As per #367 (comment), defaultProvider: 'deno' is very much not supported.

If we do want to support it, let's open a new feature request for that. jsr likely comes into the discussion at this point though?

@JayaKrishnaNamburu
Copy link
Member Author

The i think it should be changed to denoland right ?
Because i see we do have a denoland provider for the field
https://github.com/jspm/generator/blob/main/src/providers/index.ts#L55

@JayaKrishnaNamburu
Copy link
Member Author

updated it to denoland now 👍

@@ -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

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.

@guybedford
Copy link
Member

I've started some further work on jsr: support in #372. Closing for the reasons as mentioned in the comment.

@guybedford guybedford closed this Jul 21, 2024
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.

2 participants