learningtypescript / projects Goto Github PK

Hey @JoshuaKGoldberg , I just realized I didn't specify the main branch in the pull_request event of the pr title linter github action. Does it matter ? or it's okay to lint on PR raised on any branch ?

I look forward to the spam.

#10 + #19 -> a clash! 💥

Copying from #20 (comment): looks like dependabot doesn't pick up on commitlint configurations. I guess that has to be configured. https://docs.github.com/en/code-security/dependabot/dependabot-version-updates/configuration-options-for-the-dependabot.yml-file#commit-message

Tooling Report Checklist

• I have pulled the latest main branch of the repository.
• I have searched for related issues and found none that matched my request.
• This is the appropriate issue form for the tooling issue I would like to report.

Expected

Changes to package dependencies may impact the results of running test and tsc. If any package dependency in package-lock.json changes then the test.yml and tsc.yml CI actions should know to re-run all tests.

Actual

They don't, so now sometimes there's a breakage.

For example: 330567a -> https://github.com/LearningTypeScript/projects/actions/runs/3118605081/jobs/5058007718. That commit didn't change any TypeScript code but still is the first we're seeing of a failure introduced in ... some previous dependency update.

Additional Info

fyi @fuadop

Projects should have a command like (or exactly? #1) test:solutions. These should be run in CI to validate solutions all work.

Projects should be strictly consistent in their structure. A few starting ones come to mind:

• Files should match the template for their project type: README.md, package.json, jest.config.json (if it exists)
  • There should be an allowlist of instruction types in the README, starting with: "Jest", "TypeScript compile"
• No package-lock.json should exist in a child directory
• index.ts files should be a strict starting match of their solution.ts files

Some -perhaps all?- of this can likely be done with a combination of ESLint and Markdownlint. If not, I may have to write some custom npm scripts.

Before You File a Bug Report Please Confirm You Have Done The Following...

• I have pulled the latest main branch of the repository.
• I have searched for related issues and found none that matched my request.
• This is the appropriate issue form for the tooling issue I would like to report.

Expected

Right now the README.md only includes a badge for the all-contributors count.

Actual

Let's add badges there! At the very least, the relevant tools right now are:

• GitHub Actions ("build passing")
• Markdownlint
• Prettier
• TypeScript

Additional Info

See https://github.com/JoshuaKGoldberg/TypeStat for badge formatting. We can ignore the npm badge as this repo doesn't (yet?) publish to npm.

https://github.com/LearningTypeScript/projects/runs/6563248688?check_suite_focus=true

echo "⬆️ build(deps-dev): bump ts-jest from 28.0.0 to 28.0.2" | npx commitlint
/home/runner/work/_temp/dafcdb41-af6[5](https://github.com/LearningTypeScript/projects/runs/6563248688?check_suite_focus=true#step:5:6)-4290-b1f3-5d0b31ab7832.sh: line 1: syntax error near unexpected token `('

@fuadop looks like PR titles that include a ( are breaking bash escaping. This should be a one-line fix to add " quotes around the title. Do you want to take this on? 🙂 (no worries if you don't have time, I can too!)

Bug Report Checklist

• I have tried restarting my IDE and the issue persists.
• I have pulled the latest main branch of the repository.
• I have searched for related issues and found none that matched my issue.
• This is the appropriate issue form for the bug I would like to report.

Expected

typescript watch mode to successfully run for text-processor project, but it throws an error in the index.test.ts file

Actual

error TS2345: Argument of type '{ width: number; } | { align: string; width: number; }' is not assignable to parameter of type 'AlignmentOptions'.

Impacted Project

projects/arrays/text-processor/src/index.test.ts

Additional Info

No response

Bug Report Checklist

• I have tried restarting my IDE and the issue persists.
• I have pulled the latest main branch of the repository.
• I have searched for related issues and found none that matched my issue.
• This is the appropriate issue form for the bug I would like to report.

Expected

Class MockHorror is assignable to exported class Horror.

Actual

Got an error Argument of type 'MockHorror' is not assignable to parameter of type 'Horror & Horror'. Type 'MockHorror' is missing the following properties from type 'Horror': #consumed, #consume, doBattle, getPower when I run the command npm run tsc -- --watch in the the-shape-of-types project.

Impacted Project

projects/classes/the-shape-of-types

Additional Info

• I tried copying the solution.ts code into index.ts to see if there was something wrong with my solution, but the error persisted.

Tooling Report Checklist

• I have pulled the latest main branch of the repository.
• I have searched for related issues and found none that matched my request.
• This is the appropriate issue form for the tooling issue I would like to report.

Expected

Most projects use Jest tests for their .test.* file. That means TypeScript won't type check those files. There should be some additional action/workflow that runs in CI to make sure, if the solution script were used in place of the index file, tsc would report no errors.

Actual

Nothing verifies right now in CI.

Additional Info

Note that just running tsc on .//**/solution .//**/test, excluding node_modules/, will give compile errors because test files import from index files. There'll need to be some logic to copy solution files over to replace index files.

There are a few good libraries floating around. I should a relatively popular one and try it out.

This might be a no-go, if it requires adding a ton of words to an allowlist.

Tooling Report Checklist

• I have pulled the latest main branch of the repository.
• I have searched for related issues and found none that matched my request.
• This is the appropriate issue form for the tooling issue I would like to report.

Expected

Although the emojis are cute, I haven't seen other projects use them. And they can be annoying for contributors to type out.

Actual

I personally have been irked by having to figure out the right gitmoji for various PR types.

Should gitmoji be removed from the commitlint config?

Additional Info

cc @fuadop

Bug Report Checklist

• I have tried restarting my IDE and the issue persists.
• I have pulled the latest main branch of the repository.
• I have searched for related issues and found none that matched my issue.
• This is the appropriate issue form for the bug I would like to report.

Expected

The description in https://github.com/LearningTypeScript/projects/blob/733a0535a6eb504156e1beb69e88174d4fa14f5d/projects/using-ide-features/typearium/README.md should be completed and understandable.

Actual

I think I neglected to finish it...

Impacted Project

Using IDE Features > Typearium

Additional Info

No response

Typo Report Checklist

• I have pulled the latest main branch of the repository.
• I have searched for related issues and pull requests and found none that matched my issue.
• I have searched the word or phrase in question online to verify it's incorrect.
• This is the appropriate issue form for the typo I would like to report.

Impacted Project File

type-modifiers\modifiers-of-the-types\02-artifact-assertions\README.md

What's Wrong?

this readme refers to a function called announceArtifact instead of getArtifactType.

Additional Info

No response

Tooling Report Checklist

• I have pulled the latest main branch of the repository.
• I have searched for related issues and found none that matched my request.
• This is the appropriate issue form for the tooling issue I would like to report.

Expected

🧠 💡 idea: #123 added npm run tsc tests to all/changed projects, after copying solution files over original files. Let's add the same thing with npm run test!

Actual

Nothing added yet.

Additional Info

No response

Before You File a Bug Report Please Confirm You Have Done The Following...

• I have pulled the latest main branch of the repository.
• I have searched for related issues and found none that matched my request.
• This is the appropriate issue form for the tooling issue I would like to report.

Expected

There should be instructions on how to develop there.

Actual

It's mostly empty, but for a TODO.

Additional Info

This one is partially blocked on #1.

Tooling Report Checklist

• I have pulled the latest main branch of the repository.
• I have searched for related issues and found none that matched my request.
• This is the appropriate issue form for the tooling issue I would like to report.

Expected

Commits should be auto-formatted with Prettier on git commit. It's annoying for contributors to have to manually format.

Actual

This isn't (correctly) set up yet.

Additional Info

https://prettier.io/docs/en/precommit.html

As a followup to #2: use fancy Git detection techniques to understand what files have been changed from the base commit. Then only run the impacted files from the corresponding projects.

• Any root .js* file should necessitate a run of all solution scripts
• Any project-specific file should necessitate a run only of its solution scripts

See #41 > https://github.com/LearningTypeScript/projects/runs/6645184094?check_suite_focus=true:

• narrow-trails is missing a jest.config.js
• primitive-cooking uses tsc, not test

Tooling Report Checklist

• I have pulled the latest main branch of the repository.
• I have searched for related issues and found none that matched my request.
• This is the appropriate issue form for the tooling issue I would like to report.

Expected

It should be runnable directly as an executable.

Actual

You need to chmod +x it first.

Additional Info

As discussed in #63.

Thanks to @fuadop's #41, we can see that there are a ton of test failures in the vacation-planning project solution. Oops. 😂

https://github.com/LearningTypeScript/projects/runs/6643622234?check_suite_focus=true

Pr title lint fails for dependabot prs , they're missing type. see #29, #30, #31

Similar to #17, but for linting .md docs. I've historically used https://github.com/DavidAnson/markdownlint but have a sneaking suspicion there's at least one other good equivalent.

As long as it verifies basic linting goodness such as heading level increments, I'm happy.

Typo Report Checklist

• I have pulled the latest main branch of the repository.
• I have searched for related issues and pull requests and found none that matched my issue.
• I have searched the word or phrase in question online to verify it's incorrect.
• This is the appropriate issue form for the typo I would like to report.

Impacted Project File

classes\classifying-creatures\03-hamster-helpers\README.md

classes\classifying-creatures\02-dog-displays\index.ts

classes\classifying-creatures\03-hamster-helpers\README.md

What's Wrong?

classes\classifying-creatures\03-hamster-helpers\README.md: SmallPetFoot should be SmallPetFood.

classes\classifying-creatures\02-dog-displays\index.ts: Comments at top of file are referring to previous exercise; should refer to puppies instead of parrots.

classes\classifying-creatures\03-hamster-helpers\README.md: file descriptions at bottom still refer to "Puppy" code

Additional Info

No response

🐛 fix: ...
🚀 feat: ...

I love it. I want it.

Tooling Report Checklist

• I have pulled the latest main branch of the repository.
• I have searched for related issues and found none that matched my request.
• This is the appropriate issue form for the tooling issue I would like to report.

Expected

Solutions / test (pull_request) are taking as long as a minute to run all solutions. Even though they'll mostly be sped up by #3 (#63), builds that have to re-run all of them will still take a while.

https://blog.joshuakgoldberg.com/jest-babel-to-swc/ -- using SWC can get some very nice perf speedups on tests. Let's try it out?

Actual

Jest tests still use Babel.

Additional Info

No response

Read -> https://github.com/LearningTypeScript/projects/runs/6594975169

@JoshuaKGoldberg Looks like the commit message prefix of dependabot has to be changed to something shorter 🥲 . The prefix has a limit of 15 characters.

Probably we change the gitmoji on the prefix to something shorter like :art:.

Give me money, please and thank you. 💸

Before You File a Bug Report Please Confirm You Have Done The Following...

• I have pulled the latest main branch of the repository.
• I have searched for related issues and found none that matched my request.
• This is the appropriate issue form for the tooling issue I would like to report.

Expected

If someone sends a PR that doesn't match PULL_REQUEST_TEMPLATE.md's format, a CI step should complain. I'm thinking this can be similar to the PR title check, but for the body of the PR.

Actual

Nothing is set up now.

Additional Info

No response

Once #4 and #5 are in, they'll need to be documented.

Following #10: it should mention that just PR titles need to be lint-y. Commits themselves are fine to be nonsensical.

@fuadop, I have to admit, I wasn't expecting people to find this repository so quickly. And I definitely didn't expect anybody to send PRs. Much appreciated, but now I worry that there's not much to do yet for you! 😅

Most issues I assigned to myself because I expected to have to resolve them myself. But I un-assigned a bunch just now and marked them as 'accepting PRs'.

Are there other repository maintenance things that could be done? If so: let's file some issues!

Typo Report Checklist

• I have pulled the latest main branch of the repository.
• I have searched for related issues and pull requests and found none that matched my issue.
• I have searched the word or phrase in question online to verify it's incorrect.
• This is the appropriate issue form for the typo I would like to report.

Impacted Project File

generics/treasure-hunter/README.md
generics/treasure-hunter/solution.ts

What's Wrong?

Mismatching description inside Readme file.

Each Buried object can be one of three things:

• Need to check solutions in order to see there is another 4th possible type for Buried<T> - Treasure<T>.

Mismatching property names:

• Readme file points to type Catacomb<T> with shape {contents: Buried<T>, type: 'catacomb'}, but in solution.ts instead of contents is used inside as a property.
• Readme file points to type TunnelSystem<T> with shape {tunnels: Buried<T>[], type: 'tunnels'}, but in solution.ts instead of tunnels is used entrances as a property.

Additional Info

Thank you for the book <3

Before You File a Bug Report Please Confirm You Have Done The Following...

• I have pulled the latest main branch of the repository.
• I have searched for related issues and found none that matched my request.
• This is the appropriate issue form for the tooling issue I would like to report.

Expected

Chapter project directories should be in a format consumable by https://github.com/JoshuaKGoldberg/learning-typescript.

Actual

Not all chapters exist yet.

Additional Info

Filing this as a tracking issue for myself. I'll be sending multiple PRs over time to work on it.

...using the fancy new issue forms! Yes!

I checked the reviews on https://learning.oreilly.com/library/view/learning-typescript/9781098110321/ and found this one:

mxrss on September 18, 2022

The problem is mainly that the exercises don't jell well with the content in the book. I mean it might be better titled as the Javascript Developer Guide to Typescript or something like that. The main problem is the the following

The problem is that it goes from 0 to 100 really quick -- better thought out test cases, more help on using vscode to setup a test environment etc would make it a better read.

I think there are two issues here:

• The reviewer came in thinking this book would teach JS
• The projects don't do a good job of ramping up on fundamental VS Code <> TypeScript skills

The former I can't help with much at this point. It's mentioned in the opening chapter but I can understand how it'd be easy to miss. Something to think about if we do a second edition.

The latter is a very good point and something I'd like to address. I think it'd be a good idea to add a first project that's relatively simple, and walks the user through using VS Code's TypeScript support.

In general I preferred working in repos that have all "source" files nested in some easy directory structure. Commonly that's src/ for JS/TS projects, or packages/ for monorepos.

On the other hand, right now this repo just has directories for each chapter, like arrays/. If you want to do a wildcard include on, say, all project package.jsons, you'd have something like ./*/*/package.json... which would also catch packages in node_modules/. You'd need to exclude node_modules/ manually. Not ideal. 😬

Nesting chapter folders in a a projects/ directory would probably would make #2 and similar tasks easier for not having to deal with node_modules/ excludes.

cc @fuadop in case you have a preference 🙂

The link to the contributing.md file in the pull request template leads to a 404 page.. aparrently because of the exclustion of the .github in the url.

Overview

Filing a todo for myself: I plan on adding a third appetizer under https://www.learningtypescript.com/type-modifiers/modifiers-of-the-types that exercises type predicates.

This isn't accepting PRs, just using the issue tracker so I don't forget. 🙂

Tooling Report Checklist

• I have pulled the latest main branch of the repository.
• I have searched for related issues and found none that matched my request.
• This is the appropriate issue form for the tooling issue I would like to report.

Expected

Folks who contribute to the repo should be mentioned in the README.md

Actual

Recent ones haven't been. I have been neglectful.

Additional Info

As posted by @xavdid in #174, thanks!

Bug Report Checklist

• I have tried restarting my IDE and the issue persists.
• I have pulled the latest main branch of the repository.
• I have searched for related issues and found none that matched my issue.
• This is the appropriate issue form for the bug I would like to report.

Expected

The first test case returns false in 5 rolls instead of 6.

Given the phrasing in the README.md of the narrow-trails project:

After the dice roll actions are completed, decrease both food and water by 1. If either is 0 then return false.

In particular the is 0. I'd expect the first test to fail after 5 rolls of 1, not 6. The phrasing would be more accurate to say if either is less than 0 then return false, but given the games objective (and phrasing) makes sense, I think we'd actually want to update the tests to reduce the roll counts.

Actual

The first test fails in 6 rolls instead of 5.

Impacted Project

Project: unions-and-literals/narrow-trails

See the files: index.test.ts and solution.ts.

Additional Info

• I only documented the first test case.
• This would require updating the solution file.

Apologies ahead of time if I've just misunderstood the exercise in some way.

Typo Report Checklist

• I have pulled the latest main branch of the repository.
• I have searched for related issues and pull requests and found none that matched my issue.
• I have searched the word or phrase in question online to verify it's incorrect.
• This is the appropriate issue form for the typo I would like to report.

Impacted Project File

All step README.md files within Template Literal Type Shenanigans.

What's Wrong?

They're clones of other step README.mds -- they don't have any new info! 😱

Additional Info

Reported by https://twitter.com/k_stratis -- thank you! 💖

I don't like how the first project folks work on involves classes. TypeScript isn't a class-oriented language - but there's still sometimes a misconception that it is. I'd like to add a first step at the beginning of The Typeinator that involves common day-to-day things:

• var -> let / const
• function () { -> () => {
• Array and object ...s

Overview

Filing a todo for myself: I plan on adding desserts to more chapters. Hopefully everything in Functions on.

This isn't accepting PRs, just using the issue tracker so I don't forget. 🙂

.js, .json, .md, .ts, .tsx, and .yml should all be Prettier-ed. And the Github Actions builds should verify it. 🧹

Bug Report Checklist

• I have tried restarting my IDE and the issue persists.
• I have pulled the latest main branch of the repository.
• I have searched for related issues and found none that matched my issue.
• This is the appropriate issue form for the bug I would like to report.

Expected

Some tests should pass based on the project's specification, examples and other tests results.

Actual

Some tests fail due to the expected result being incorrect.

Impacted Project

Arrays -> Text Processor

Additional Info

In projects/arrays/text-processor/src/index.test.ts there are some inconsistencies in the tests expected results.

For example:

On the test case at lines 31 to 38 we have:

[
  ["ab de", "abc def"],
  { width: 4 },
  [
    ["ab  ", "de  "],
    ["abc ", "def "],
  ],
],

We can see that the first element of the texts array "ab de" gets split in the expected result: ["ab ", "de "],.

This is also true for the immediate next test at lines 39 to 47

[
  ["ab de", "abc def", "abcd ef"],
  { width: 4 },
  [
    ["ab  ", "de  "],
    ["abc ", "def "],
    ["abcd", "ef  "],
  ],
],

We can see the first element of the text array "ab de" also gets split in the expected result: ["ab ", "de "].

However, in the test following this one, the expected result is different doesn't follow this pattern. On line 48 we have:

[["ab de", "abc def"], { width: 5 }, [["ab de"], ["abc  ", "def  "]]],

In this case, the first element of the text array "ab de" doesn't get split, instead we have that its corresponding expected result is ["ab de"] which differs from the previous test cases and ,to my understanding, also differs from the specification of this project.

A gave a quick look an this "inconsistencies" also appear in more test cases like the test on line 49:

[["abc def", "abc def"], { width: 8 }, [["abc def "], ["abc def "]]],

test on line 66:

[["abc def", "abcdefghi"], { width: 8 }, [["abc def "], ["abcdefghi"]]],

or tests at lines 73 to 77

[["abc def"], { align: "left", width: 7 }, [["abc def"]]],
[["abc def"], { align: "left", width: 8 }, [["abc def "]]],
[["abc def"], { align: "left", width: 9 }, [["abc def  "]]],
[["abc def"], { align: "left", width: 10 }, [["abc def   "]]],
[["abc def"], { align: "left", width: 11 }, [["abc def    "]]],

These "inconsistencies" as I'm calling them right now, in turn make some tests fail which I believe shouldn't.

I re-read the project description and specification a couple times but couldn't find anything that suggested this behavior was expected/intentional. An apologize in advance if my reading comprehension baamboozled me.

https://github.com/all-contributors/all-contributors is a nice looking tool. I'd like to use it.

Thanks to #10 there's already at least one person to thank 😄

Basically, this: https://prettier.io/docs/en/precommit.html#option-2-pretty-quickhttpsgithubcomazzpretty-quick. When folks make a commit locally, Prettier should run on files to auto-format them.

Tooling Report Checklist

• I have pulled the latest main branch of the repository.
• I have searched for related issues and found none that matched my request.
• This is the appropriate issue form for the tooling issue I would like to report.

Expected

Steps to reproduce are found in one example System of a Clown

I believe it would be best for the errors to be scoped to the current exercise. I observe that one can do this with include in tsconfig

There are a number of examples of this in the repo already it seems.

Actual

I just tried this fresh and observe Found 137 errors

Errors span the gamut and include things like

• 01-shallow-equality/index.ts' is not a module
• objects/the-typer/src/solution.ts 'treasure' was also declared here
• Property 'around' does not exist on type
• using-ide-features/temporary-code/index.test.ts Cannot find name 'logFavoritePersons'

Additional Info

No response