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

feat: allow file editing and updates the preview #762

Draft
wants to merge 22 commits into
base: better-playground
Choose a base branch
from

Conversation

6ri4n
Copy link

@6ri4n 6ri4n commented Nov 1, 2024

What changed / motivation ?

WIP Implementing Playground Container

Changed the following web container project structure:

  • Adjusted the build tool to use Vite instead of Rollup
  • Supports hot reloading and JSX syntax
  • Add generateCSS.js script which is make-stylex-sheet.js but with small changes

Changed the following to Playground.js:

  • Allow file editing to app.jsx
  • Preview gets updated

Linked PR/Issues

#732

Additional Context

In Playground.js, if app.jsx changes, the function updateFiles writes the changes to the app.jsx file and then runs the generateCSS.js script which creates stylex.css inside the src directory.

The app.jsx file includes import "./stylex.css"; and if removed then any further changes to the styling won't be applied, it'll use the last stylex.css file that was created.

Screenshots:

Container Running with Vite
stylex-1

Hot Reloading / Applying Changes
stylex-2

Pre-flight checklist

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Nov 1, 2024
@6ri4n 6ri4n marked this pull request as draft November 1, 2024 06:20
@6ri4n
Copy link
Author

6ri4n commented Nov 1, 2024

Hi @nmn, here’s what I have so far. I'd appreciate any feedback or suggestions.

@nmn
Copy link
Contributor

nmn commented Nov 1, 2024

The code makes sense, but it doesn't seem to be working in my testing. See for yourself:

https://stylex-git-fork-6ri4n-dev-better-playground-fbopensource.vercel.app/playground/

Changing the styles just causes the box to lose all styles. It seems like the new CSS file isn't being loaded after being generated. Even if I manually reload the iFrame the new styles are not being loaded. I'll try to pull your PR locally and investigate a bit further.

@nmn
Copy link
Contributor

nmn commented Nov 1, 2024

I pulled the changes down locally and the file update is working correctly, so is the generateCSS script. However, the web preview is broken. I only saw the new styles applied once after a dozen attempts. Most of the time, the CSS simply does not load.

Copy link
Contributor

@nmn nmn left a comment

Choose a reason for hiding this comment

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

The generateCSS script from the docs app is probably not the best approach here.

The best idea would be to setup a standalone Rollup + React + StyleX application and then once that works correctly, re-create that setup within the playground.

Our Rollup plugin is the most reliable, so it should work well.

We can also try a custom Vite plugin, which is what is set up right now and can do Fast Refresh. There are a few examples of Vite plugins used with frameworks like Svelte, Qwik and Remix. One of those should work well as a starting point.

apps/docs/components/Playground.js Outdated Show resolved Hide resolved
apps/docs/components/Playground.js Outdated Show resolved Hide resolved
Comment on lines 27 to 28
flowSyntaxPlugin,
jsxSyntaxPlugin,
Copy link
Contributor

Choose a reason for hiding this comment

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

The StyleX plugin should already be doing this.

apps/docs/components/Playground.js Outdated Show resolved Hide resolved
apps/docs/components/Playground.js Outdated Show resolved Hide resolved
@nmn
Copy link
Contributor

nmn commented Nov 1, 2024

This branch will also need to be rebased again. I just rebased the better-playground branch.

@6ri4n
Copy link
Author

6ri4n commented Nov 6, 2024

@nmn, I don't think I have the abilities to get the Rollup setup to work. I found that excluding --watch in the rollup command allowed JSX to work but I'm not sure how to get it to re-bundle without --watch. I've also tried rearranging the order of the plugins in rollup.config.mjs and also used rollup-plugin-livereload for hot reloading but kept running into different errors.

Can you also take a look at the latest deployment, it seems to be working for me. If the latest deployment is also working for you, would you still want me to try and setup a custom Vite plugin?

@nmn
Copy link
Contributor

nmn commented Nov 6, 2024

It does mostly work for me as well. Every edit of the file refreshes the preview twice, which is what I saw as well.

Regarding a custom Vite plugin, I have many examples on the Ecosystem page that should work well. I will try to get a standalone project going and make sure it works outside of a WebContainer first. I'll try Rollup first because it's easier to setup.

@nmn
Copy link
Contributor

nmn commented Nov 6, 2024

@6ri4n

I have a project with Rollup working properly here:
https://stackblitz.com/edit/sb1-tu4v6l?file=src%2FApp.tsx

  • It's a basic React app with Rollup and livereload
  • It uses a forked version of @stylexjs/rollup-plugin
  • It adds a few lines of code to write the individual transformed JS files and the collected styles from them to a previews folder.

Using this project, you should be able to have a four tab design with the following tabs:

  • The editor
  • The transformed JS code of the currently active file
  • The styles collected from the currently active file
    • OR the final CSS
  • The preview
    • The preview should "just work".
  • It should not refresh twice on every change

You can probably skip the eslint and typescript parts of the project for now. That should make the setup fairly small.


I made a mistake and shared this on the other PR yesterday.

- note: rollup command not generating dist directory and associated files
@6ri4n
Copy link
Author

6ri4n commented Nov 8, 2024

@nmn, I adjusted the project to exclude typescript and eslint and got the entire files section in a standalone project working. I used the rollup babel plugin and placed it before the rollup-stylex-plugin otherwise the following error would occur:
(plugin rollup-plugin-stylex-playground) RollupError: Expression expected
src/App.jsx

However, only when running the project inside the web container it's unable to create the dist and previews directory and its associated files (even after creating the directory beforehand) which prevents the preview to work.

@nmn
Copy link
Contributor

nmn commented Nov 9, 2024

This is strange because the link I shared is running on Stackblitz which runs within a webcontainer itself...

@6ri4n
Copy link
Author

6ri4n commented Nov 11, 2024

This is strange because the link I shared is running on Stackblitz which runs within a webcontainer itself...

I'm not sure what I'm doing wrong, do you have any suggestions on what I should do?. I also tried setting up the project to include typescript and eslint, matching the structure in the stackblitz link you shared but I got the same issue of the dist directory not being created.

@nmn
Copy link
Contributor

nmn commented Nov 11, 2024

@6ri4n let me try it on my end and get back to you. Maybe there are some config options needed for webcontainers.

@6ri4n
Copy link
Author

6ri4n commented Nov 12, 2024

Do you have any suggestions on other good first issues I can possibly work on in the meantime?

I also found that when the container first loads, the preview is black but when you modify the jsx and let it apply the changes the preview shows the results however it's not behaving as intended.

@6ri4n
Copy link
Author

6ri4n commented Nov 17, 2024

@nmn, I got the preview working by adding back the setTimeout in the 'server-ready' listener to wait for the project to fully load. However, currently it's set to 5 seconds and works on Chrome but I found that on other browsers like FireFox it requires a longer delay, otherwise the preview will be black.

The preview also renders twice for some reason (you can see this if you remove the setIsUpdating(true); and setIsUpdating(false); in the handleCodeChange function), do you have any ideas on why this is happening?

I added a setTimeout that setIsUpdating(false); with a delay of 5 seconds to wait for the re-bundling process to finish. But the user experience isn't good due to the 5 second loading screen, I was thinking we could do the same as StackBlitz where there's no loading text so the web container serves whenever it's done re-bundling. What are your thoughts on this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants