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): specify import versions (avoids Deno warnings) #8

Closed
wants to merge 1 commit into from

Conversation

rivy
Copy link

@rivy rivy commented Oct 9, 2022

The current version causes Deno to report warnings when imported...

$ deno run -q -r deno\async.ts
Warning Implicitly using latest version (0.159.0) for https://deno.land/std/path/mod.ts
$ deno run -q -r deno\sync.ts
Warning Implicitly using latest version (0.159.0) for https://deno.land/std/path/mod.ts

This pins the version to the current Deno std (0.159.0) for both scripts and removes the warnings.

$ deno run -q -r deno\async.ts
$ deno run -q -r deno\sync.ts

@lukeed
Copy link
Owner

lukeed commented Oct 9, 2022

it shouldnt be pinned. otherwise we're forcing deno users to pin/hold onto a stale version for basic path-module imports

@rivy
Copy link
Author

rivy commented Oct 9, 2022

Deno best practice recommends that all imports be versioned, hence the warning from deno that I noted above.

I understand the desire to "keep current". But, as deno doesn't concern itself with semantic versioning for imports, using floating "latest" versions will eventually cause breakage for an imported module. And pinning in this module doesn't affect any other module; it only makes sure that this module will continue to work as written today.

For any production grade application, all the dependencies must be versioned. There is no exception to this. Keeping it un-versioned would download the latest code, and it may not be compatible with current application code.

Never use un-versioned dependencies

ref: https://medium.com/deno-the-complete-reference/dependency-management-in-deno-48f1c91ad84d

... it is a warning that you are implicitly using a version which is risky. You are welcome to ignore the warning.

however, it's fairly standard practice to import standard libraries using the latest version isn't it?

Not really. All imports should always be versioned.

ref: denoland/deno#10822

@rivy
Copy link
Author

rivy commented Oct 18, 2022

@lukeed , no further discussion?

@rivy
Copy link
Author

rivy commented Nov 4, 2022

@lukeed , I believe the problem you are concerned about doesn't exist.
But if you're not comfortable with the change, I'll recommend vendoring a corrected version for yargs.

@rivy
Copy link
Author

rivy commented Jan 29, 2023

☠️ ⚰️

@rivy rivy closed this Jan 29, 2023
rivy added a commit to rivy-fix/escalade that referenced this pull request Nov 20, 2023
- specify import versions per best practice and to avoid Deno warnings
- pin to std@0.134.0 to avoid Deno prompts

# refs

- ref: [fix(deno): specify import versions (avoids Deno warnings)](lukeed#8)
  - closed as dead after being ghosted by @lukeed
- ref: [fix(deno): refactor to avoid prompts during module import](yargs/yargs#2217)

## related discussion/issues

[fix(node): Make global.ts evaluate synchronously](denoland/std#2098)
[Execution order of imports in deno is unclear/unexpected](denoland/deno#14243)
[std/node should avoid TLA](denoland/std#2097)
[HowTO test that a module is *no-panic* and *no-prompt* when statically imported?](denoland/deno/issues/#15356)

[Discussion ~ Bring back permission prompt behind a flag](denoland/deno/issues/#3811)
[Security prompt by default (instead of throw)](denoland/deno/issues/#10183)
[Seeking a better UX for permissions](denoland/deno/issues/#11061)
[Design Meeting 2021-07-29 ~ `Prompt by default`](denoland/deno/issues/#11767)
[permission prompt problems](denoland/deno/issues/#11936)
[`deno repl` has permissions by default?](denoland/deno/issues/#12665)
[Bad UX with prompt by default](denoland/deno/issues/#13730)
[DENO_NO_PROMPT env var support](denoland/deno/issues/#14208)
[feat: Add DENO_NO_PROMPT variable](denoland/deno/issues/#14209)
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