Skip to content

Implement use_location - #721

Merged
rmorshea merged 21 commits into
mainfrom
spa
Apr 8, 2022
Merged

Implement use_location#721
rmorshea merged 21 commits into
mainfrom
spa

Conversation

@rmorshea

@rmorshearmorshea commented Apr 1, 2022

Copy link
Copy Markdown
Collaborator

By providing access to a use_location hook, users can implement primitive routing. The catch is that, at present, state will not be persisted as users navigate between routes. Support for that will have to come in a future update.

@rmorshea
rmorshea marked this pull request as ready for review April 2, 2022 18:24
@rmorshearmorshea changed the title Turn IDOM Client into an SPAEnable Basic RoutingApr 4, 2022
@rmorshea
rmorshea requested a review from ArchmongerApril 4, 2022 05:10
@Archmonger

Archmonger commented Apr 4, 2022

Copy link
Copy Markdown
Contributor

Can you shoot me a sample snippet of the syntax for this so I can test?

@rmorshea

rmorshea commented Apr 4, 2022

Copy link
Copy Markdown
CollaboratorAuthor

@Archmonger here's a demo of how it should work:

fromidomimporthtml, component, runfromidom.serverimportstarletteasserver@componentdefShowLocation():
loc=server.use_location()
returnhtml.h1(loc.pathname+loc.search)
run(ShowLocation, implementation=server)

@ArchmongerArchmonger left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Might want to rename the PR to "use_location hook" since this isn't routing, but rather location propagation.

Routing = integrated support for rendering different components based on regex path

Comment threadrequirements/pkg-deps.txt
Comment threadsrc/client/package.json Outdated
Comment threadsrc/client/packages/idom-app-react/package.json
Comment threadsrc/client/packages/idom-client-react/src/server.js Outdated
Comment threadsrc/client/snowpack.config.js
Comment threadsrc/idom/server/flask.py
Comment threadsrc/idom/server/flask.py
Comment threadsrc/idom/server/flask.py
Comment threadsrc/idom/server/utils.py Outdated
@rmorshearmorshea changed the title Enable Basic RoutingImplement use_locationApr 4, 2022
Comment threadsrc/idom/server/default.py
if we poll sync then the server might not ever
get a hold of the event loop. this was causing
a problem for the sanic server in a test
@rmorshea

Copy link
Copy Markdown
CollaboratorAuthor

@Archmonger, any idea why this test is failing on Windows?

@Archmonger

Archmonger commented Apr 5, 2022

Copy link
Copy Markdown
Contributor

The way that test is written, I wouldn't expect it to raise anything on any OS?

From what I can tell,

  1. Create /dir/file.txt
  2. Create /escaped-file.txt via symlink
  3. Check if /dir/file.txt is safe

Why would you expect /dir/file.txt to be unsafe?

@Archmonger

Archmonger commented Apr 5, 2022

Copy link
Copy Markdown
Contributor

Scratch that, I see you're trying to symlink the empty /dir/file.txt to a /escaped-file.txt.

I don't think this is a valid test case to be honest. If the user intentionally symlinked a file into our dir, we should consider it safe. In general, symlinks/hardlinks should be considered as "real files" within both their given directories.

If you want this to fail for some reason, you'd need to use os.path.realpath() or Path.absolute() to follow the symlink to it's origin.

Comment threadsrc/idom/server/utils.py Outdated
@rmorshea
rmorshea merged commit e43bcc0 into mainApr 8, 2022
@rmorshea
rmorshea deleted the spa branch April 8, 2022 02:54
Sign up for freeto 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.

2 participants

@rmorshea@Archmonger