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

Improve path management in wws #3

Closed
Angelmmiguel opened this issue Oct 10, 2022 · 3 comments · Fixed by #56
Closed

Improve path management in wws #3

Angelmmiguel opened this issue Oct 10, 2022 · 3 comments · Fixed by #56
Assignees
Labels
🚀 enhancement New feature or request

Comments

@Angelmmiguel
Copy link
Contributor

The wws server relies on the local filesystem heavily. The way we are managing paths at this point may cause issues when running the server in Windows environments. In addition to that, the way we check for multiple extensions is quite complex and error prone.

The goal for this issue is to:

  • Define available extensions in a constant (in the future we may mek this configurable)
  • Limit the conversion of paths to String and &str to printing data
  • Rely on Path and PathBuf for any path management requirement
@assambar
Copy link
Contributor

If the handlers folder is a flat one and I don't specify it with a trailing / I get 404 on the handler.

Given

$> tree /apps
/apps
└── handler.js
0 directories, 1 file

If I do this

wws /apps

I get no response to curl localhost:8080/handler in the shell and see a 404 in the server logs

[2022-10-18T14:55:01Z INFO  actix_web::middleware::logger] 127.0.0.1 "GET /handler HTTP/1.1" 404 0 "-" "curl/7.68.0" 0.000067

However, if I add an / at the end of apps it all works.

wws /apps/

@Angelmmiguel Angelmmiguel added the 🐛 bug Something isn't working label Nov 14, 2022
@Angelmmiguel Angelmmiguel changed the title Improve path management in wws Fix path management in wws Nov 14, 2022
@Angelmmiguel Angelmmiguel removed the 🐛 bug Something isn't working label Nov 14, 2022
@Angelmmiguel Angelmmiguel changed the title Fix path management in wws Improve path management in wws Nov 14, 2022
@Angelmmiguel
Copy link
Contributor Author

@assambar thanks for the report! The issue will be fixed in #30 😄

@Angelmmiguel
Copy link
Contributor Author

The main issue we have here is the limitations of the glob library. The library doesn't support patterns with multiple extensions like **/*.{js,wasm}. We can chain multiple paths (iterators), but if the extensions are not hardcoded, we will need to flatten all the paths while we're reading the files.

The wax library is more up-to-date and covers the different use cases. I will switch to use this one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🚀 enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants