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

fix: deno import from skypack #283

Merged
merged 2 commits into from
May 11, 2022

Conversation

baoshan
Copy link
Contributor

@baoshan baoshan commented May 3, 2022

This PR should fix this issue.

I’m not sure if import type alone is sufficient. It’s likely this PR will not be merged because there’s no maintainer currently. Anyway here it is.

@ghost ghost added this to Inbox in JS May 3, 2022
@wolfy1339 wolfy1339 added the Type: Bug Something isn't working as documented label May 4, 2022
@ghost ghost moved this from Inbox to Bugs in JS May 4, 2022
@wolfy1339
Copy link
Member

It’s likely this PR will not be merged because there’s no maintainer currently.

There are some community maintainers that help around, though no full time active maintainer.

Thanks for the PR!

Can you instead put aws-lambda as an optionalDependency? I don't think it makes sense to always install it whenever a user installs this package

@baoshan baoshan force-pushed the type_only_import_aws_lambda branch from 62d78d7 to c632013 Compare May 4, 2022 03:11
@baoshan
Copy link
Contributor Author

baoshan commented May 4, 2022

Thanks @wolfy1339 for the suggestion!

It’s optional now. I’m interested if import type alone (without optionalDependency) is enough to silent the error. Will make some experiments when I have time. Actually skypack and esm.sh behave differently, it may be a long way to go before a satisfying Deno experience.

Since there are undergoing changes to Octokit.js, I guess (some of) the current repos will phase out. Maybe it’s best to be patient for now.

@wolfy1339
Copy link
Member

Let's leave it as-is for now, merge this PR and revisit this if it becomes a problem again

@wolfy1339 wolfy1339 merged commit 71c952d into octokit:master May 11, 2022
JS automation moved this from Bugs to Done May 11, 2022
@github-actions
Copy link

🎉 This PR is included in version 3.6.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Bug Something isn't working as documented
Projects
No open projects
JS
  
Done
Development

Successfully merging this pull request may close these issues.

Deno import issue auth-oauth-app
2 participants