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

Remove PERSEUS_STANDALONE #87

Closed
arctic-hen7 opened this issue Dec 12, 2021 · 11 comments
Closed

Remove PERSEUS_STANDALONE #87

arctic-hen7 opened this issue Dec 12, 2021 · 11 comments
Assignees
Labels
A-cli Area: command line interface C-enhancement Category: enhancement D-medium Difficulty: medium P-high Priority: high S-in-development Status: in development

Comments

@arctic-hen7
Copy link
Member

Is your feature request related to a problem? Please describe.
I just spent 10 minutes trying to figure out why deployment had failed, only to realize that I hadn't provided PERSEUS_STANDALONE=true, and I built this project. If I'm having trouble remembering that this environment variable needs to be provided, it's probably an issue!

Describe the solution you'd like
The PERSEUS_STANDALONE environment variable should be removed in favor of some other method of telling Perseus that it's been deployed. My initial instinct is to have a feature on the crates that need it that gets enabled at the command-line level when we deploy, otherwise maybe something with build scripts might work.

Describe alternatives you've considered
Continuing to use the environment variable, but I think it's a pain point and an unnecessary requirement.

Additional context
Add any other context or screenshots about the feature request here.

@arctic-hen7 arctic-hen7 self-assigned this Dec 12, 2021
@arctic-hen7 arctic-hen7 added A-cli Area: command line interface C-enhancement Category: enhancement D-medium Difficulty: medium P-high Priority: high S-in-development Status: in development labels Dec 12, 2021
@phaleth
Copy link
Contributor

phaleth commented Dec 12, 2021

PERSEUS_STANDALONE=true needs to be provided right before execution of the server binary, not before build, export or deploy, and as far as I remember it only makes sense to provide the var after perseus deploy.

If you have no need to do perseus deploy often enough you might not remember to set the var, but most importantly it is well documented.

@arctic-hen7
Copy link
Member Author

I know, but I'm pretty sure this can all be done with features, so I'm working on that at the moment to make things more convenient.

@phaleth
Copy link
Contributor

phaleth commented Dec 12, 2021

Heh, ok. I guess maybe it's too much trouble to weed out PERSEUS_STANDALONE.

I remember having trouble with figuring out I need to set HOST=0.0.0.0, but now that I think of it, I find it to be a good thing for local development in case of the Perseus developer is on an un-trusted network. Not sure HOST still needs to be set with Warp.

Is Warp now also used for local development? Guess I should see the docs.

@arctic-hen7
Copy link
Member Author

Yes, Warp is now used for everything (not explicitly clear in the docs, but mentioned in the CHANGELOG). Yes, HOST still needs to be set with Warp, and come to think of it I should mention in the docs that Warp doesn't support setting localhost as the host, you have to use 127.0.0.1 (the new default).

Also, @phaleth are you still planning to write a page in the docs on using Perseus with Docker? I think it'd be a great addition and given the sheer amount of work you've done with the two together I think you'd be the best person to write it!

@phaleth
Copy link
Contributor

phaleth commented Dec 12, 2021

I still do plan to document Perseus deployment with Docker. Actually, sorry about hijacking this issue, but thanks for bringing that up.

So far I didn't even manage to figure out how to contribute to Perseus, cause I'm not done with the deployment story yet. In short the next thing I need is to figure out how to transpile out the dynamic JS import() function out of the bundle.js. I know for instance webpack takes care of the import() function by default but webpack is too much of a burden that shouldn't be introduced to a modern project even if only deployment phase is considered. So, I'll prolly look for some cli tool and let you know at some point.

What consumes my time lately is figuring out the basics of how WebAssembly works, which makes me mostly, as far as I can tell so far, doubt Rust, but I'll get there. Also I don't really know Rust, which is kind of a blocker, but I'll get there as well, just not sure when, haha.

Anyway, @arctic-hen7, since this issue has been hijacked, the main question I have for you about Perseus is that: Do you think now that actix-web is not so deeply rooted inside Perseus that a hot reload feature for development is possible to implement such that it comes with Perseus OOTB?

@phaleth
Copy link
Contributor

phaleth commented Dec 12, 2021

Sorry, actually, it's the import keyword that is causing trouble.

image

@arctic-hen7
Copy link
Member Author

The PERSEUS_STANDALONE environment variable is no longer required! This will be released in the next beta version along with support for Sycamore v0.7.0.

@arctic-hen7
Copy link
Member Author

@phaleth that import keyword should be able to be gotten rid of with rollup, which was originally part of Perseus v0.1.0. If you figure out how to do this, please let me know and I'll add it to the docs.

Also, on hot reload, yes. As I've said on Discord, that will be my priority for v0.3.1. Thank you for reminding me about this though, this is tracked now by #90.

@phaleth
Copy link
Contributor

phaleth commented Dec 13, 2021

@arctic-hen7, alright, yeah, thank you. Hot reload will be great and don't worry about HMR, rust code takes a while to compile anyways.

rollup is pretty capable, but sadly node based. I'll see if I can setup a portable version of esbuild and what it's capable of. Anyway, it's kinda cool that you had rollup setup here for Perseus.

esbuild should be able to take care of minification and also so called compression of the bundle.js at least. I've noticed that the Perseus documentation site uses the kind of raw/dev-mode-like/unoptimized bundle.js, so esbuild might come in handy. Nowadays, if I was gonna need to install node I'd use vite and only vite without looking back.

@phaleth
Copy link
Contributor

phaleth commented Dec 13, 2021

Ok, I got the esbuild command for bundle.js figured out. The file bundle.js has lost over 13 KB.

./package/bin/esbuild ./tiny/pkg/dist/pkg/perseus_engine.js --minify --target=es6 --outfile=./tiny/pkg/dist/pkg/perseus_engine.js --allow-overwrite

image

@arctic-hen7
Copy link
Member Author

Great! I'm going to go ahead and include that in the optimizations docs then! Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-cli Area: command line interface C-enhancement Category: enhancement D-medium Difficulty: medium P-high Priority: high S-in-development Status: in development
Projects
None yet
Development

No branches or pull requests

2 participants