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 deps (avoids Deno warnings) #464

Closed
wants to merge 3 commits into from

Conversation

rivy
Copy link

@rivy rivy commented Oct 9, 2022

The current version causes Deno to report warnings when imported.
This PR pins the version to the current Deno std (0.159.0) and removes the warnings.

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

$ git checkout fix.deno
Previous HEAD position was 3aba24c chore(main): release yargs-parser 21.1.1 (#455)
Switched to a new branch 'fix.deno'
Branch 'fix.deno' set up to track remote branch 'fix.deno' from 'origin'.

$ deno run -r deno.ts

@rivy
Copy link
Author

rivy commented Oct 29, 2022

@bcoe, review?

@bcoe
Copy link
Member

bcoe commented Nov 3, 2022

Change looks good to me, but we seem to have a regression with the rollup dependency :/

@rivy
Copy link
Author

rivy commented Nov 4, 2022

I added a commit updating rollup and associated modules. It includes a necessary change from rollup-plugin-ts to @rollup/plugin-typescript.

It builds/tests successfully for me, locally and the CI workflow, with NodeJS v12+.

@rivy rivy force-pushed the fix.deno branch 2 times, most recently from 3007efc to 236f06c Compare November 4, 2022 13:57
@rivy
Copy link
Author

rivy commented Nov 4, 2022

I revised the build changes to a minimal set that works with NodeJS v12+ and mirrored the changes to the yargs/y18n PR.
There are some notsup complaints, specifically for NodeJS v12, but everything builds and tests correctly.
If your CI passes then this should be complete.

Note, though these PR changes shouldn't have changed any coverage metrics, I am getting a coverage threshold error...

------------------------|---------|----------|---------|---------|-------------------------
File                    | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s       
------------------------|---------|----------|---------|---------|-------------------------
All files               |   99.42 |    97.82 |     100 |   99.42 |                         
 index.ts               |    88.7 |    61.53 |     100 |    88.7 | 43-49                   
 string-utils.ts        |     100 |      100 |     100 |     100 |                         
 tokenize-arg-string.ts |     100 |      100 |     100 |     100 |                         
 yargs-parser-types.ts  |     100 |      100 |     100 |     100 |                         
 yargs-parser.ts        |     100 |    98.64 |     100 |     100 | 159,633,727-729,744,953 
------------------------|---------|----------|---------|---------|-------------------------
ERROR: Coverage for lines (99.42%) does not meet global threshold (99.5%)
ERROR: Coverage for statements (99.42%) does not meet global threshold (99.5%)

... if you want to add a test.

@bcoe bcoe added the ci label Nov 11, 2022
@rivy
Copy link
Author

rivy commented Nov 23, 2022

@bcoe , as the changes here in the PR didn't actually change any of the coverage tested files, how about just changing the thresholds for line and statement %'s to "99"?

Or, maybe better, use /* c8 ignore start */ ... /* c8 ignore stop */ directives around the lines 43-49 within index.ts since they're not testable with CJS.

@rivy
Copy link
Author

rivy commented Nov 23, 2022

@bcoe , CI now passes with the added /* c8 ignore start */ ... /* c8 ignore stop */ directives.

@rivy
Copy link
Author

rivy commented Dec 7, 2022

@bcoe , review?

@rivy
Copy link
Author

rivy commented Dec 12, 2022

@bcoe , once this is merged, it and yargs/y18n#147 can be used within yargs, which should fix all Deno warnings (excepting the escalade lack of explicit versioning, ref: lukeed/escalade#8 and yargs/yargs#2217 (comment)).

When done, I'll close yargs ~ PR #2216 and just fork and publish a yargs version for Deno with a fix for the escalade problem.

@rivy
Copy link
Author

rivy commented Jan 1, 2023

@bcoe , just checking back in... I believe all CI should PASS.

@rivy
Copy link
Author

rivy commented Jan 21, 2023

Ping @bcoe.

@shadowspawn
Copy link
Member

I have not reviewed this PR but noticed the dependency updates. Just noting I have added a couple of other PRs with approaches to updating dependencies: #478 and #471

@rivy
Copy link
Author

rivy commented Nov 20, 2023

☠️ ⚰️
It's been a year; I'll move on with a fork containing the needed fixes.

@rivy rivy closed this Nov 20, 2023
@rivy rivy deleted the fix.deno branch November 20, 2023 03:12
rivy added a commit to rivy-fix/yargs-parser that referenced this pull request Nov 20, 2023
- best practice and strongly needed b/c Deno does *not* use semantic versioning
- avoids Deno warnings
- also avoids Deno prompts

# refs

- [fix(deno): specify deps (avoids Deno warnings)](yargs#464)
  - closed for non-participation at 1 yr open
- [fix(deno): refactor to avoid prompts during module import](yargs/yargs#2217 (comment))

## related discussion/issues

[refactor ~ implement *no-panic*/*no-prompt* changes](rivy/deno.dxx@a53375d) @@ <https://archive.is/Esp5e>

[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)
rivy added a commit to rivy-fix/yargs-parser that referenced this pull request Nov 20, 2023
- best practice and strongly needed b/c Deno does *not* use semantic versioning
- avoids Deno warnings
- also avoids Deno prompts

# refs

- [fix(deno): specify deps (avoids Deno warnings)](yargs#464)
  - closed for non-participation at 1 yr open
- [fix(deno): refactor to avoid prompts during module import](yargs/yargs#2217 (comment))

## related discussion/issues

[refactor ~ implement *no-panic*/*no-prompt* changes](rivy/deno.dxx@a53375d) @@ <https://archive.is/Esp5e>

[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)
rivy added a commit to rivy-fix/yargs-parser that referenced this pull request Nov 20, 2023
- best practice and strongly needed b/c Deno does *not* use semantic versioning
- avoids Deno warnings
- also avoids Deno prompts

# refs

- [fix(deno): specify deps (avoids Deno warnings)](yargs#464)
  - closed for non-participation at 1 yr open
- [fix(deno): refactor to avoid prompts during module import](yargs/yargs#2217 (comment))

## related discussion/issues

[refactor ~ implement *no-panic*/*no-prompt* changes](rivy/deno.dxx@a53375d) @@ <https://archive.is/Esp5e>

[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)
rivy added a commit to rivy-fix/yargs-parser that referenced this pull request Nov 21, 2023
- best practice and strongly needed b/c Deno does *not* use semantic versioning
- avoids Deno warnings
- also avoids Deno prompts

# refs

- [fix(deno): specify deps (avoids Deno warnings)](yargs#464)
  - closed for non-participation at 1 yr open
- [fix(deno): refactor to avoid prompts during module import](yargs/yargs#2217 (comment))

## related discussion/issues

[refactor ~ implement *no-panic*/*no-prompt* changes](rivy/deno.dxx@a53375d) @@ <https://archive.is/Esp5e>

[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)
rivy added a commit to rivy-fix/yargs-parser that referenced this pull request Nov 21, 2023
- best practice and strongly needed b/c Deno does *not* use semantic versioning
- avoids Deno warnings
- also avoids Deno prompts

# refs

- [fix(deno): specify deps (avoids Deno warnings)](yargs#464)
  - closed for non-participation at 1 yr open
- [fix(deno): refactor to avoid prompts during module import](yargs/yargs#2217 (comment))

## related discussion/issues

[refactor ~ implement *no-panic*/*no-prompt* changes](rivy/deno.dxx@a53375d) @@ <https://archive.is/Esp5e>

[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)
rivy added a commit to rivy-fix/yargs-parser that referenced this pull request Nov 21, 2023
- best practice and strongly needed b/c Deno does *not* use semantic versioning
- avoids Deno warnings
- also avoids Deno prompts

# refs

- [fix(deno): specify deps (avoids Deno warnings)](yargs#464)
  - closed for non-participation at 1 yr open
- [fix(deno): refactor to avoid prompts during module import](yargs/yargs#2217 (comment))

## related discussion/issues

[refactor ~ implement *no-panic*/*no-prompt* changes](rivy/deno.dxx@a53375d) @@ <https://archive.is/Esp5e>

[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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants