[TS] Add types to flash, clean up some logic #1824
Conversation
Krisztiaan
changed the title
Co-authored-by: Tobbe Lundberg <tobbe@tlundberg.com>
Co-authored-by: Tobbe Lundberg <tobbe@tlundberg.com>
Co-authored-by: Tobbe Lundberg <tobbe@tlundberg.com>
Co-authored-by: Tobbe Lundberg <tobbe@tlundberg.com>
There was a problem hiding this comment.
I've suggested removing a bunch of comments.
I like comments that explain potentially confusing code. Or comments that explains the intent of code, if the intent is not immediately obvious from looking at the code itself
In these cases I don't think the comments fit in those criterias. Let me know if you disagree.
And then I added brackets around an if-statement. We like to have those, even for single line if-statements.
Co-authored-by: Tobbe Lundberg <tobbe@tlundberg.com>
Co-authored-by: Tobbe Lundberg <tobbe@tlundberg.com>
Co-authored-by: Tobbe Lundberg <tobbe@tlundberg.com>
Co-authored-by: Tobbe Lundberg <tobbe@tlundberg.com>
Co-authored-by: Tobbe Lundberg <tobbe@tlundberg.com>
Co-authored-by: Tobbe Lundberg <tobbe@tlundberg.com>
Co-authored-by: Tobbe Lundberg <tobbe@tlundberg.com>
|
@Tobbe fully agree, didn't remove the comments because I was just there to make the TS change (and some other sensible mods). As for the braces for blocks, this should probably be enforced via |
💯 I was actually surprised it wasn't already a rule |
Co-authored-by: Tobbe Lundberg <tobbe@tlundberg.com>
|
The ESlint rule is added in #1836 |
There was a problem hiding this comment.
E2E tests = ✅
FYI: I found an unrelated Scaffold Flash error (see #1839). Confirming this PR keeps current behavior and Flash works on Delete.
…late-and-auth-setup-tests * 'main' of github.com:redwoodjs/redwood: [TS] Add types to flash, clean up some logic (redwoodjs#1824) Remove backticks (redwoodjs#1826) Add Gravatar to the list of Tom's accomplishments (redwoodjs#1813) Don't open storybook when --open=false is given (redwoodjs#1795) Passthrough error codes on cli failures (redwoodjs#1791) E2E: if path to project given, use the installed packages (redwoodjs#1837) Add Cypress Step6 Test (redwoodjs#1780)
Resolves: #1217
Added
allowJs: falseto web, since this was the last piece missing.This PR does not address the potential for a bug that could happen when a dismiss button on a tree that does not update with the messages array is pressed, thus potentially removing an unintended message.
Namings may seem a bit "weird" considering long standing practice of generic names such as
stateandpayload, but especially in this case, it seems to me as making it just a little bit more elegant.Disagreement welcome, as in anything else!