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

Bad URL mapping for apps that use HTML 5 routing #46

Closed
blittle opened this issue Jun 25, 2019 · 0 comments · Fixed by #47
Closed

Bad URL mapping for apps that use HTML 5 routing #46

blittle opened this issue Jun 25, 2019 · 0 comments · Fixed by #47

Comments

@blittle
Copy link
Contributor

blittle commented Jun 25, 2019

Sofe tries to support loading services with an included path:

// The service name might have a path in it!
// For example, you might `import a from 'service/a/path.js!sofe';`
// In that case, we want `getServiceName` to return just `'service'`;

See https://github.com/CanopyTax/sofe/blob/master/src/utils.js#L26

I don't know anyone who actually uses that feature (Canopy does not). The issue arises because the address variable in that code (https://github.com/CanopyTax/sofe/blob/master/src/utils.js#L14) is prefixed with the entire URL that the web-app was loaded from. So for an HTML5 push state web app, that URL could be lots of things depending on the link that loads the page. With hash routing, it is always only one thing. The logic in that code assumes that windowPath and address will always be roughly the same thing. That isn't the case, so there are cases where sofe gets confused and barfs. Again, only with HTML5 push state routing.

The question is, do I change this code to make pathed services actually work for HTML 5? Or do I remove the feature altogether? To be clear, the situation where I'm getting an error isn't using pathed services, but does break with HTML 5 push state routing.

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 a pull request may close this issue.

1 participant