Skip to content

feat: add deno entry#4

Merged
lukeed merged 5 commits intomasterfrom
feat/deno
Aug 11, 2020
Merged

feat: add deno entry#4
lukeed merged 5 commits intomasterfrom
feat/deno

Conversation

@lukeed
Copy link
Owner

@lukeed lukeed commented Aug 10, 2020

First pass at a Deno entry.
All fs.* operations are available under Deno namespace.
Only had to import std:path module.

Questions:

  1. Both Deno.* operations require the allow-read permission. Is this something I have to flag on my side?
  2. Is there a better/built-in way to go from AyncIterator to Array?
  3. Do I have to add /deno to my package.json files? Assume not since publishing works thru git releases only

//cc @bcoe


Closes #3

@codecov-commenter
Copy link

codecov-commenter commented Aug 10, 2020

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (24efb4b) to head (230627b).
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff            @@
##            master        #4   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            2         2           
  Lines           20        20           
=========================================
  Hits            20        20           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@bcoe
Copy link

bcoe commented Aug 11, 2020

Both Deno.* operations require the allow-read permission. Is this something I have to flag on my side?

I've been capturing the error, and providing a useful note for users. It sounds like there is a way to prompt for permissions, might be a good option too:

https://dev.to/bmorearty/better-deno-security-ask-for-permission-at-runtime-1fnm

Is there a better/built-in way to go from AyncIterator to Array?

I'm not sure. Your approach seemed pretty reasonable to me 👍

Do I have to add /deno to my package.json files? Assume not since publishing works thru git releases only

No, you should be able to ignore /deno.

Copy link

@bcoe bcoe left a comment

Choose a reason for hiding this comment

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

This is looking good 👍 for yargs, I will need the sync option

I think, I'd be happy to pitch in if you run out of cycles this week.

@lukeed
Copy link
Owner Author

lukeed commented Aug 11, 2020

Thanks! I'll add extra sync entry. I figured everything would be async in Deno because of top-level await but obviously that was a bad assumption.

I think permissions is all that's left to solve. Catching it makes sense for a yargs but I don't think it would for every module down the tree.

Happy to focus on it this week. Would hate to be the blocker for your conversion effort

@bcoe
Copy link

bcoe commented Aug 11, 2020

@lukeed yes, let me know where you land on permissions, this is something I'm trying to learn to navigate too.

@lukeed
Copy link
Owner Author

lukeed commented Aug 11, 2020

Hey, so I asked the Deno Discord and the recommendation is to allow the permission errors to happen. Any required permissions should be listed in the README (or in the functions' jsdoc blocks), and it's ultimately up to the consumer to add the appropriate flags. This is also how you can get method/usage-driven granular permissions.

I found feature proposals in the Deno repo about adding manifests/import-map-like files to define the permissions ahead of time, but there's not been any action taken on them yet. Design still WIP

@lukeed lukeed merged commit b47f528 into master Aug 11, 2020
@lukeed lukeed deleted the feat/deno branch August 11, 2020 05:01
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.

Feature request: Compatibility with Deno

3 participants