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

ci(): codesandbox #8135

Merged
merged 210 commits into from
Mar 11, 2023
Merged

ci(): codesandbox #8135

merged 210 commits into from
Mar 11, 2023

Conversation

ShaMan123
Copy link
Contributor

@ShaMan123 ShaMan123 commented Aug 8, 2022

add codesandbox ci to fabric for issues/PR and for prototyping (even on your local machine!)
https://codesandbox.io/docs/ci

I think I managed to install the github ci app on fabric..
if not please do

The issue you (@asturur ) had of codesandbox not opening should be resolved by using this tool because it should build the repo behind scenes with their engine and expose it for sandboxes to use (instead of containing a build file inside the sandbox)

  • .codesandbox

    • ci.json - generic config
    • templates - a folder containing templates to create a sandboxes from
    • deploy.js - deploys the template to codesandbox
  • scripts - exposed sandbox cmd with build start deploy

npm run sandbox deploy /path/to/sandbox

npm run sandbox deploy -- --template node
> created codesandbox https://codesandbox.io/s/fgh476

npm run sandbox build node
> building node sandbox at ~/.fabric/vanilla

npm start
> start a sandbox from a list of available templates

Try It

image

@ShaMan123 ShaMan123 added the CI/CD label Aug 8, 2022
@ShaMan123 ShaMan123 requested a review from asturur August 8, 2022 19:08
@asturur
Copy link
Member

asturur commented Aug 9, 2022

Give me time to read it so i learn something new. In general i don't have much to say on this PR, i ll merge it for sure, i just want to read it


const DEV_MODE = process.env.NODE_ENV === "development";

export function useCanvas(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

we may want to simplify this method
I took it from existing code

Copy link
Contributor Author

@ShaMan123 ShaMan123 Aug 9, 2022

Choose a reason for hiding this comment

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

for example saveState isn't so good because if code changes in the init block it doesn't take effect on hot reload, but only on hard reload

So maybe I should remove it entirely

It will be bad if the sandbox raises bugs because it is supposed to be a clean, safe bug repro w/o interference....

Copy link
Contributor Author

@ShaMan123 ShaMan123 left a comment

Choose a reason for hiding this comment

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

added a deploy script

so you can test the templates by running

npm run dev sandbox -- --template node/next/vanilla

OHH this will make prototyping so much easier!

@asturur
Copy link
Member

asturur commented Mar 11, 2023

@ShaMan123 since our last chat i have blocked with the back and i can barely sit.
I merging this as is, i have NO IDEA what does it do, but i understand is needed for debugging local devl at this point and hopefully is way better than using fabricjs.com.

@asturur
Copy link
Member

asturur commented Mar 11, 2023

Merging knowing this may have killed node 14

@asturur asturur merged commit ed6466f into master Mar 11, 2023
@asturur asturur deleted the sandbox-ci branch March 11, 2023 14:04
@ShaMan123
Copy link
Contributor Author

Sorry to hear about your back

The description is not good enough, for sure.
Exposed 3 templates under .codesandbox: next, node, vanilla
And added the sandbox command to the cli.
The command starts the sandbox locally, linking it to the fabric local repo and is able to deploy it to codesandbox (unlinked, need to do that).
This is also intended to work with Sandbox CI app. The app should post a comment on PRs with the templates linked to the PR or something like that.

Merging knowing this may have killed node 14

please explain. I didn't expect this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants