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

chore: Backport much of #793 #1073

Merged
merged 6 commits into from
Feb 12, 2024
Merged

Conversation

rschristian
Copy link
Member

Vast majority of this is Marvin's. Had some great stuff sitting around in the WMR PR that I wanted to nab

Does 3 major things:

  1. Switches to preact-iso for routing, lazy loading.
  2. Gets off unistore to Context for most items
  3. Breaks up the reliance on controllers/page and creates separate DocPage, Blog, etc. Much cleaner, much simpler.
    • Tutorial & REPL could use a similar treatment but they'd be a bigger refactor

I tried to keep the diff down but... this is gonna be a big one. Sorry.

Shouldn't be any functional differences on the site.

Co-authored-by: Marvin Hagemeister <hello@marvinh.dev>
@@ -88,12 +90,11 @@
"node-fetch": "^2.6.1",
"preact": "10.15.1",
"preact-custom-element": "^4.3.0",
"preact-iso": "2.2.0",
Copy link
Member Author

@rschristian rschristian Feb 5, 2024

Choose a reason for hiding this comment

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

Something in 2.3.0+ (I'm guessing preactjs/wmr#864) results in onLoadEnd never being called on first page rendered, so our our loading bar is in the "loading" state until the user switches pages.

Comment on lines +10 to +11
//console.error(`Unknown language: ${lang}`);
return code;
Copy link
Member Author

@rschristian rschristian Feb 5, 2024

Choose a reason for hiding this comment

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

We're missing bash & diff, both are rather inconsequential (throwing is now an actual problem, just not one worth addressing right now, hence the comment out)

Comment on lines +222 to +228
// TODO: Webpack creates a circular dependency that
// it cannot resolve. Temporarily disabled
//useEffect(() => {
// if (meta && meta.next) {
// getContent([lang, meta.next]);
// }
//}, [meta && meta.next, url]);
Copy link
Member Author

Choose a reason for hiding this comment

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

As the comment says, unfortunately lazy loading the tutorial creates a circular dep that Webpack is not a big fan of. I couldn't figure out any way to stop it from dying.

Suggestions certainly welcome if you got 'em.

Comment on lines +3 to +5
// TODO: SolutionProvider should really just wrap the tutorial,
// but that requires a bit of refactoring
import { SolutionProvider } from './controllers/tutorial/index.js';
Copy link
Member Author

@rschristian rschristian Feb 5, 2024

Choose a reason for hiding this comment

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

As noted, this is a bit weird due to the current state of the tutorial page. Really need to refactor it into a few different components w/ a shell, but haven't yet gotten to it.

@rschristian rschristian merged commit bd23e1b into master Feb 12, 2024
5 checks passed
@rschristian rschristian deleted the refactor/backport-wmr-changes branch February 12, 2024 06:40
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.

2 participants