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

Update 4-the-shell-scripts.md #1167

Merged
merged 1 commit into from
Oct 30, 2022
Merged

Update 4-the-shell-scripts.md #1167

merged 1 commit into from
Oct 30, 2022

Conversation

RobK
Copy link
Contributor

@RobK RobK commented Oct 30, 2022

Fixing 2 minor issues I encountered while following tutorial:

  • Got an error when trying to use the format: 'email'. Removed this and it worked fine. Perhaps the real solution is to fix the ajv-format issue inside FoalTS? But this PR is enough let people proceed with the tutorial.
  • Fixed minor spacing typo.

Issue

Running the command from the tutorial (foal run create-user email="john@foalts.org" password="john_password" name="John") generates a CLI error:

C:\workspaces\foal-react-tuto\backend-app>foal run create-user email="john@foalts.org" password="john_password" name="John"

C:\Users\Robert\AppData\Roaming\npm\node_modules\@foal\cli\node_modules\ajv\dist\vocabularies\format\format.js:63
                throw new Error(unknownMsg());
                      ^

Error: unknown format "email" ignored in schema at path "#/properties/email"    
    at unknownFormat (C:\Users\Robert\AppData\Roaming\npm\node_modules\@foal\cli\node_modules\ajv\dist\vocabularies\format\format.js:63:23)
    at validateFormat (C:\Users\Robert\AppData\Roaming\npm\node_modules\@foal\cli\node_modules\ajv\dist\vocabularies\format\format.js:50:17)
    at Object.code (C:\Users\Robert\AppData\Roaming\npm\node_modules\@foal\cli\node_modules\ajv\dist\vocabularies\format\format.js:22:13)
    at keywordCode (C:\Users\Robert\AppData\Roaming\npm\node_modules\@foal\cli\node_modules\ajv\dist\compile\validate\index.js:454:13)
    at C:\Users\Robert\AppData\Roaming\npm\node_modules\@foal\cli\node_modules\ajv\dist\compile\validate\index.js:222:17
    at CodeGen.code (C:\Users\Robert\AppData\Roaming\npm\node_modules\@foal\cli\node_modules\ajv\dist\compile\codegen\index.js:439:13)
    at CodeGen.block (C:\Users\Robert\AppData\Roaming\npm\node_modules\@foal\cli\node_modules\ajv\dist\compile\codegen\index.js:568:18)
    at iterateKeywords (C:\Users\Robert\AppData\Roaming\npm\node_modules\@foal\cli\node_modules\ajv\dist\compile\validate\index.js:219:9)
    at groupKeywords (C:\Users\Robert\AppData\Roaming\npm\node_modules\@foal\cli\node_modules\ajv\dist\compile\validate\index.js:200:13)
    at C:\Users\Robert\AppData\Roaming\npm\node_modules\@foal\cli\node_modules\ajv\dist\compile\validate\index.js:192:13

Node.js v18.12.0

Solution and steps

Checklist

  • Add/update/check docs (code comments and docs/ folder).
  • Add/update/check tests.
  • Update/check the cli generators.

@LoicPoullain
Copy link
Member

Hi @RobK 👋

Thank you for reporting the issue!

This is indeed a regression introduced in v3.0.0 which uses version 8 of AJV. AJV v6 includes formats, v8 doesn't. Hence the error.

I opened a PR to fix this use. It will be released with v3.0.1.

Could you update the PR to revert back the change on the format? Then, we will be able to merge it to fix the import typo.

Thank you for having reporting this 👍 👍

@LoicPoullain LoicPoullain mentioned this pull request Oct 30, 2022
1 task
@RobK
Copy link
Contributor Author

RobK commented Oct 30, 2022

Hi @LoicPoullain, I updated this PR to remove the format related changes and left in the other typo fix

Copy link
Member

@LoicPoullain LoicPoullain left a comment

Choose a reason for hiding this comment

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

Thank you!

@LoicPoullain LoicPoullain merged commit 414c066 into FoalTS:master Oct 30, 2022
Issue tracking automation moved this from Work In Progress to Done / Closed This Release Oct 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Issue tracking
  
Done / Closed This Release
Development

Successfully merging this pull request may close these issues.

None yet

2 participants