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

docs: added cheatsheet for best practices #2266

Merged
merged 65 commits into from
Dec 22, 2022

Conversation

reemardelarosa
Copy link
Contributor

@reemardelarosa reemardelarosa commented Nov 23, 2022

What is it?

  • Feature / enhancement
  • Bug
  • Docs / tests

Description

Add Cheatsheet: Best Practices created by Misko

Use cases and why

Screenshot 2022-12-08 at 5 43 38 PM

Checklist:

  • My code follows the developer guidelines of this project
  • I have performed a self-review of my own code
  • I have made corresponding changes to the documentation
  • Added new tests to cover the fix / functionality

@stackblitz
Copy link

stackblitz bot commented Nov 23, 2022

Review PR in StackBlitz Codeflow Run & review this pull request in StackBlitz Codeflow.

@KenAKAFrosty
Copy link
Contributor

Hey thanks a ton for doing this!

Also just wanted to point out the "Or Habits...." and "Or Dehydrate...." sentences just under the main header were more internal notes I think? Can probably just leave those off.

@reemardelarosa
Copy link
Contributor Author

@KenAKAFrosty updated

---

# No-Hydration Tips
## Don't register events eagerly with `useClientEffect$()`
Copy link
Member

Choose a reason for hiding this comment

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

How about changing this to Don't register DOM events eagerly with useClientEffect$()`? I was confused until I realized it was about DOM

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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



## EXCEPTION
When doing SSG for pure static files, this is a necessary evil. The server won't have current location information when the server generates the files at build time.
Copy link
Member

Choose a reason for hiding this comment

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

I don't understand this. useLocation() gets the location of the page that is being SSR'ed, no? And then when it runs on the client it gets the actual file location? Doesn't it still work with SSG?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

yeah, i dont understand this exception either, useLocation() will also work in SSG, SSG is the same as SSR (just cached forever)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@manucorporat
Copy link
Contributor

I am not sure we should call this Anti-patterns, bit negative? how about Best practices?

@reemardelarosa
Copy link
Contributor Author

@KenAKAFrosty @mhevery do we have a resolution for the comments here from @manucorporat nad @wmertens ?

@mhevery
Copy link
Contributor

mhevery commented Dec 7, 2022

Please change it to Best Practices and let's get this merged in.

@reemardelarosa reemardelarosa changed the title docs: added cheatsheet for anti patterns docs: added cheatsheet for best practices Dec 8, 2022
manucorporat and others added 19 commits December 8, 2022 09:33
clock.css is not correctly loaded which causes the clock to not show.
Error: Cannot find module './clock.css?inline' or its corresponding type declarations.
When using onError in the IDE as provided in the example it gives the following error:
Property 'onError' does not exist on type 'IntrinsicAttributes & ResourceProps<string[]>'

The solution says it should be onRejected
Co-authored-by: Jeremy Wickersheimer <jwickers@gmail.com>
utherpally and others added 27 commits December 8, 2022 09:33
Co-authored-by: langbamit <langbamit@gmail.com>
Qwik array map in qwik vs. react did not have key even though in rendering portion of docs it says
key is required
Co-Authored-By: Tran Thien Khiem <20198928+tuoitrevohoc@users.noreply.github.com>
To avoid running into unexpected `wasm-pack` failures when doing full build, adds note to Contributing docs about installation of `wasm-pack`, linking to site showing installation command for each platform.
@reemardelarosa
Copy link
Contributor Author

@mhevery @manucorporat @KenAKAFrosty updated to Best Practices

@reemardelarosa
Copy link
Contributor Author

@manucorporat @KenAKAFrosty can we merge this PR or do we need to update more?

cc: @mhevery

@mhevery mhevery merged commit eec2096 into QwikDev:main Dec 22, 2022
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.

None yet