Skip to content

Conversation

@rmorshea
Copy link
Collaborator

@rmorshea rmorshea commented Oct 20, 2022

closes: #796

This is low priority, but unfortunately it involves some API changes that I'd like to get out of the way. Currently, the server exposes the following interface:

/<prefix>/<path>/_api/stream
/<prefix>/<path>/_api/modules/<file-path>
/<prefix>/<path>/<file-path>

This lead to some complicated route patterns. Under these changes though, the server exposes a simpler interface:

/_idom/assets/<file-path>
/_idom/stream/<spa-path>
/_idom/modules/<file-path>
/<spa-path>

Here, all of IDOM's core server APIs are housed under a /_idom root URL. This makes it simpler for the client to direct requests to the correct location and, I assume, makes it possible for the browser to cache static resources. The only complication is that the client needs to coordinate requesting /_idom/stream/<spa-path> where <spa-path> is the same from /<spa-path> where the SPA is being hosted. Backend implementations should then normalize <spa-path> when constructing Connection.location by stripping any URL prefix.

Checklist

Please update this checklist as you complete each item:

  • Tests have been included for all bug fixes or added functionality.
  • The changelog.rst has been updated with any significant changes.
  • GitHub Issues which may be closed by this Pull Request have been linked.

@rmorshea rmorshea marked this pull request as ready for review October 29, 2022 20:58
@rmorshea
Copy link
Collaborator Author

rmorshea commented Oct 29, 2022

@acivitillo, I don't know if you're still using IDOM, but if you are, this may impact you. If you're using a url_prefix of /_idom you should ok - route priorities should work correctly.

@rmorshea rmorshea force-pushed the vite branch 3 times, most recently from efe272b to d53ad83 Compare October 29, 2022 23:00
Copy link
Contributor

@Archmonger Archmonger left a comment

Choose a reason for hiding this comment

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

LGTM.

@rmorshea rmorshea merged commit 8ff09d2 into main Oct 31, 2022
@rmorshea rmorshea deleted the vite branch October 31, 2022 04:19
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.

Snowpack is Deprecated

3 participants