-
Notifications
You must be signed in to change notification settings - Fork 237
Added a golden reference for a web application using the geo crate. #1480
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
base: main
Are you sure you want to change the base?
Conversation
15981dc to
b598159
Compare
Just for my own understanding, can you provide an example of something that passes my CI check in #1478 but would fail the later wasmpack build step? (Regardless, I'm still personally interested in having a simple functioning wasm example.) |
1) Added a landing page image to the web-app README.md. 2) Pass the design throught a HTML validator, fixed all my mistakes. 3) Fixes minor issues with the documentation.
b598159 to
a914e70
Compare
Thanks for pick me up on this... After playing around with this for a while ... I can confirm I was mistaken following that PR and running cargo check -p geo --lib --target wasm32-unknown-unknown --no-default-featuresdoes expose all the current problems I see |
|
No worries - I still think it makes sense to include a simple example to point people to. To avoid a future where this example is broken, I think we should also include the build process in CI, so we can know if our example is working. As you've said elsewhere, rust on the web is a shifting sands. I don't think we should endeavor to provide the "state of the art" or the "best" way of doing things today, but rather optimize for simplicity and stability, so that this is less likely to be a maintenance burden in the future. |
1) Added a landing page image to the web-app README.md. 2) Pass the design throught a HTML validator, fixed all my mistakes. 3) Fixes minor issues with the documentation.
michaelkirk
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm personally interested in merging this. How do other maintainers feel?
The only thing I'd like to see before merging it is adding a CI job to make sure the build actually functions.
michaelkirk
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Whoops - didn't mean to "approve" until it's added to CI.
|
I'm neutral to slightly against this. I like that this provides an easy way to test that WASM integration is working with |
|
That's a fair point about ongoing maintenance just to keep the dependencies updated from security issues. It's unfortunate that the "hello world" of a "minimal web app" these days is a 5000 line package-lock.json With the fix in #1492, geo will once again compile on wasm. So using geo in the browser works just like any rust crate. I don't think we get much value in shouldering the burden of maintaining a generic "here's the best way to build rust for the browser". There are general purpose tutorials for that. The one thing that's kind of special about geo is where this all started. We use A more ambitious example of showing off geo's functionality in the browser might be more worthwhile. Maybe that's well served by a project like rgis and we can link to it as an example of how to run geo in the browser. |
Regarding the recent break moving from geo 0.31 to 0.32 (#1476)
There was a long discussion of how to configure for wasm*-unknown-unknown targets
michaelkirk response was we are missing a github action to cover that case. #1478
My response to that is .. its complicated, some compile time assert fail only on a later wasmpack build steps.
We need a pre-issue to build a golden reference which will detail all the build steps which could break when pulling a source of reference from the browser sandbox environment.
Here is the golden reference
CHANGES.mdif knowledge of this change could be valuable to users.