Make paths relative #73

icidasset · 2020-02-20T17:24:57Z

Addresses #15 and #69

How it works

Every generated HTML file contains a base element.

index.html                           # <base href="" />
nested/index.html                    # <base href="../" />
another/nested/index.html            # <base href="../../" />

Works no matter where I put these files:
http://localhost:3000/, https://example.io/sub-directory/ or http://127.0.0.1:8080/ipns/example.io/.

Example

# ROOT = https://example.io/sub-directory/
# FILE = nested/index.html
<base href="../" />
<a href="another/nested/">Another Nested Page</a>

The href in base will always point to /sub-directory/. And the href in links, src in scripts and images, etc. will always have a path that reflect the path in your project. So in other words, if we want to link to another/nested/, this "relative" link will be the same on every page we have, because, the base element makes it point to the right thing.

Notes

Anchor links will need a page prefix as well (ie. current/page#id-on-page)
Base element behaviour can be overridden using an absolute path (ie. starts with /)
Things loaded before the base element are loaded as if the base element was not defined. Meaning, <link rel="preload" href="content.json" as="fetch" crossorigin />, defined before the base element, will load the content from the current directory.

Issues

~~I'm (still) having issues with the "static http" stuff. I'm not sure my changes here broke anything relating to that.~~
I'm noticing that the service worker loads weird paths (eg. http://localhost:3000/nested-page//content.json). Notice the double // before content.json. I'm not sure if that's because of my changes or just a weird workbox/service-worker thing.
~~Icon paths in manifest.json are incorrect, they should not be prefix with assets/. Still figuring out how to fix this.~~
~~Switching between pages doesn't work yet. It tries to load http://content.json/.~~

Testing

Tested with personal project of mine
~~Couldn't compile example projects in this repo because of static http issues (see above)~~
Tests are passing

Other solutions

@cyberglot suggested a different solution, using the canonical url as a prefix for the urls. While that works great for a single domain, this requires you to compile a different version for each domain and directory you want to put your site on. Which may work for a lot of people, but not for everyone. For example, our company uses IPFS which has a browser extension that caches your site offline through a proxy. So, when you use the offline version, through the proxy, the site is on a different domain and path. We can't specify a different build for the proxied version, so it's broken in at least one place using this solution.

Another solution would be to have elm-pages make every generated path relative. This might be better because you would avoid the use of the base element, but I think implementing this would be more difficult. I could be wrong though.

Thoughts?

cyberglot · 2020-02-21T14:15:00Z

I will have a look later today, thanks por mentioning me.

cyberglot · 2020-02-21T17:20:21Z

@icidasset I checked Travis, and I can try to help you get the tests passing, but not sure if you want/need help.

@dillonkearns sorry for acting like a maintainer here.

icidasset · 2020-02-21T17:32:16Z

@cyberglot I would love that 👍 There's still a few things broken here, trying to figure it out as I go along here 🤷‍♂ (Keeping track of every issue in the main post above under "Issues")

Sidenote: I'm currently keeping another branch based on this one, which I use in projects for work. Basically the same as this, but has the compiled Main.js file in it. Mainly doing this to properly test this code in production.

# Using it in package.json like so:
"elm-pages": "icidasset/elm-pages#fdf09810a6e9e321763058d4b7c14996707fd2a5",

And then using elm-git-install for the elm code. You can use that code too if you want.

icidasset · 2020-02-21T22:36:15Z

Ok, I definitely messed up something with the StaticHttp stuff, I'm not sure what's going on. Any ideas @dillonkearns ?

icidasset · 2020-02-26T16:37:52Z

Fairly confident I fixed most of the issues with this PR. If someone else has time to test this, please do 🙏

dillonkearns · 2020-03-03T06:40:31Z

Hello @icidasset and @cyberglot, sorry for the slow response. It's awesome to see you two collaborating and coordinating on this. Thanks so much for working on this feature! 🙏

I tried it out locally, and it's mostly looking like a good design and like it's working properly. The one thing I'm noticing is the note about og tags. Many of these <meta> tags require that you have an absolute URL, otherwise they won't work (don't ask me why, I've tried to get around it by they seem to insist 😑).

See the note about OpenGraph tags in the MDN docs you linked to: https://developer.mozilla.org/en-US/docs/Web/HTML/Element/base

Open Graph tags do not acknowledge , and should always have full absolute URLs.

Any ideas on what to do about that? Is there a way to make OpenGraph tags happy using the base tags strategy?

icidasset · 2020-03-03T13:42:51Z

@dillonkearns Awesome ☺️ Regarding the og tags, I think it's best to go with @cyberglot's solution there, and prefix the path with the canonical url.

Should I make those changes or do you want to tackle that?

dillonkearns · 2020-03-03T15:37:00Z

@icidasset that makes sense. Actually, canonical URLs are a special thing when it comes to SEO. There needs to be exactly one. From what I've read, Google can easily flag your site as spam, or seriously penalize your site ranking, if it sees that you have duplicate content. So it's important for a page to have exactly one canonical URL, not one canonical URL for each place you can access it. So it's a feature, not a bug, to have the canonical URL be hardcoded and be exactly one URL.

In production, you can see that the og tags are already using the canonical URL as a base. So I don't think any more work is needed there, we just need to revert it to use the canonical URL. And then I think that users will need to use the full URL including their base path for the canonical URL in the Pages.Platform.application config. Does that all make sense and sound right to you two?

For reference, here are the current og tags on elm-pages.com. It's being generated from this canonical URL:

elm-pages/examples/docs/src/Main.elm

Lines 636 to 638 in bb8bb38

    
           canonicalSiteUrl : String 
        
           canonicalSiteUrl = 
        
               "https://elm-pages.com"

.

Also, you can see how I'm doing that in the head tags here. I'm automatically grabbing the canonical URL when I render the <head> tags using this logic:

https://github.com/dillonkearns/elm-pages/blob/master/src/Head.elm#L211-L212

So again, I think that all we need to do as have users include their base path in the canonical URL, and then restore that functionality and we should be good.

icidasset · 2020-03-03T17:00:09Z

Aye sounds good! I'll take a look at fixing that asap.

icidasset · 2020-03-10T15:33:49Z

@dillonkearns The canonical urls should be back for the seo/head tags. Any other changes I should make?

dillonkearns · 2020-03-11T05:17:21Z

Hey @icidasset, I'm still seeing errors on the Netlify build, and my local build is stalling at 98%. Any idea what might be going on?

Here's an example netlify build. I cleared the cache and ran this one, so this is a clean build and still getting an error: https://app.netlify.com/sites/elm-pages/deploys/5e6873cfd9b8f78594a17401

icidasset · 2020-03-11T16:09:43Z

Ok, sorry about that. Everything should be good now 😅

dillonkearns · 2020-03-11T16:22:26Z

🎉 That's awesome, can't thank you enough for the hard work! Thanks for sticking with it even though it got tricky! 😁

I'll take a look today 👍

icidasset · 2020-03-27T18:37:06Z

@dillonkearns Could it be that the Netlify stuff is cached and therefor showing errors? Travis build has passed, so 🤷‍♂ Also, have you had a chance to look at this? 🙏

dillonkearns · 2020-03-27T19:15:18Z

Hello @icidasset! I tried a clear cache and rebuild on Netlify, and it's still getting this error 🤔

https://app.netlify.com/teams/dillonkearns/builds/5e7e4ffbbbc75201c0ce2c58

12:13:00 PM: Compiling ...
12:13:00 PM: -- NAMING ERROR
12:13:00 PM: --------------------------------------------------- src/Main.elm
12:13:00 PM: I cannot find a `String.chop` variable:
12:13:00 PM: 64|
12:13:00 PM:       |> String.chop "/"
12:13:00 PM:                ^^^^^^^^^^^
12:13:00 PM: The
12:13:00 PM:  `String` module does not expose a `chop` variable. These names seem close
12:13:00 PM: though:
12:13:00 PM:     String.cons
12:13:00 PM:     String.map
12:13:00 PM:     String.all
12:13:00 PM:     String.any
12:13:00 PM: Hint: Rea
12:13:00 PM: d <https://elm-lang.org/0.19.1/imports> to see how `import`
12:13:00 PM: declarations work in Elm.
12:13:00 PM: -- NAMING ERROR ------------------------------
12:13:00 PM: --------------------- src/Main.elm
12:13:00 PM: I cannot find a `String.chop` vari
12:13:00 PM: able:
12:13:00 PM: 39|         |> String.chop "/"
12:13:00 PM: ^^^^^^^^^^^
12:13:00 PM: The
12:13:00 PM:  `String` module d
12:13:00 PM: oes not ex
12:13:00 PM: pose a `chop` variable. These names seem close
12:13:00 PM: though:
12:13:00 PM:     String.co
12:13:00 PM: ns
12:13:00 PM:     String.map
12:13:00 PM:     String.all
12:13:00 PM:     String.any
12:13:00 PM: Hint: Read <https://elm-lang.org/0.19.1/imports> to see how `import`
12:13:00 PM: declarations work in Elm.
12:13:00 PM: 
Detected problems in 1 module.
12:13:00 PM: npm
12:13:00 PM:  ERR! code ELIFECYCLE
12:13:00 PM: npm ERR!
12:13:00 PM:  errno 1
12:13:00 PM: npm ERR!
12:13:00 PM:  elm-pages@1.2.10 build: `cd generator && elm make src/Main.elm --output src/Main.js --optimize`
12:13:00 PM: npm ERR! Exit status 1
12:13:00 PM: npm
12:13:00 PM: ERR!
12:13:00 PM: npm
12:13:00 PM: ERR! Failed at the elm-pages@1.2.10 build script.
12:13:00 PM: npm ERR!

Any ideas?

I'll spend some time testing it out this weekend, would love to get this released 👍

icidasset · 2020-03-28T15:33:15Z

generator/src/Main.elm

@@ -57,11 +57,16 @@ dropIndexFromLast path =
        |> List.reverse


+chopForwardSlashes : String -> String
+chopForwardSlashes =
+    String.split "/" >> List.filter ((/=) "") >> String.join "/"


FYI, I want to use the Pages.Internal.String module here, but don't know how to share that code with the generator/cli.

Ah yes. I'll keep that in mind for the future. It's pretty minor so this seems good for now to me 👍

…ow the URL resolves. Uses https://developer.mozilla.org/en-US/docs/Web/API/URL/URL.

dillonkearns · 2020-03-28T21:18:44Z

@icidasset I pushed a couple of commits fixing the two issues I found.

Link prefetching

The link prefetching wasn't working because it was checking the hrefs in anchor tags against the base urls, which no longer had leading slashes. To make it more robust, I added this commit which resolves the URLs to check if they are the same or not:

e10c062

`unknown` in base url

I was seeing unknown for the base url at the root-level.

I added this commit to fix that (not sure if there's a cleaner way to do that). 6649ce7

One small code change

This seemed a little cleaner to let the browser tell us how it resolves the base url (taking the <base> tag into account, rather than querying for the base tag. I think it's exactly the same, but when I look at this code it feels more trustworthy somehow 😄

964f9af#diff-168726dbe96b3ce427e7fedce31bb0bcL42-R42

Does the `<base>` tag need to update on page changes?

I'm noticing we're not changing the <base> tag when we navigate to new pages. So it is technically out of sync if you, for example, navigate from the root page (<base href="./">) to the blog index page (still <base href="./">, but it should be <base href="../">). If you refresh the page on the blog page, it will now be <base href="../">. But it doesn't change on JS-based navigations because we're only changing it in the pre-rendered HTML, not in JS.

I'm not seeing any behavior that's broken by this. Do you think it's good the way it is, or do you think there could be any issues related to this? I guess it's just interpreting the <base> tag on the initial page load, so it would just ignore it anyway if we change it, so it's not worth changing it with JS?

Let me know what you think of these changes! Otherwise, everything is looking great and ready to merge I think!

icidasset · 2020-03-28T21:57:13Z

Thanks for all the code changes @dillonkearns !
Looks great 👍

I guess it's just interpreting the tag on the initial page load

As far as I know when using history.pushState, it does reference the original base tag and behaves accordingly. So yeah, I'm also guessing this is the case. We would need to dive into the W3C specs to make sure this is the case 👀

dillonkearns · 2020-03-28T22:01:40Z

As far as I know when using history.pushState, it does reference the original base tag and behaves accordingly. So yeah, I'm also guessing this is the case.

Yeah, I agree 👍 From my testing, that seems to be exactly what it's doing. And if I run document.baseURI before and after a JS-based navigation, it does indeed keep the correct baseURI. So that's another indication that the browser has the correct understanding of the base, based on the original page load.

So yeah, I think we're in good shape on that! I'll prepare this for release 👍 Thanks again for your hard work on this! And thank you @cyberglot for your work and for getting the ball rolling on this functionality!

icidasset · 2020-03-28T22:04:45Z

I got this from the spec regarding pushState:

Resolve the value of the third argument, relative to the entry script's base URL.

So I guess this confirms our assumption 👍
Source: https://www.w3.org/TR/2011/WD-html5-20110113/history.html

I'll prepare this for release 👍

🎉 🙌

dillonkearns · 2020-03-28T23:01:17Z

relative to the entry script's base URL

Good find! So perhaps it could even cause problems to try to change it within the JS. That makes sense, sounds like our current setup is perfect 👍

icidasset requested a review from dillonkearns February 20, 2020 17:51

icidasset force-pushed the base branch from 18fb01d to 91e0176 Compare February 21, 2020 16:03

icidasset force-pushed the base branch from 81c2658 to d8f9232 Compare February 21, 2020 21:51

icidasset force-pushed the base branch 4 times, most recently from e00e9db to b724504 Compare February 26, 2020 14:37

icidasset force-pushed the base branch from 6ad9562 to 162b2ae Compare March 10, 2020 15:28

icidasset added 2 commits March 10, 2020 16:31

Make urls relative using the base element

1a5175d

Fix statichttp and url-change issues

8f44ddf

icidasset force-pushed the base branch from 162b2ae to 285a989 Compare March 10, 2020 15:31

Use canonical urls in seo tags

dec9b43

icidasset force-pushed the base branch from 285a989 to dec9b43 Compare March 10, 2020 15:32

icidasset added 3 commits March 11, 2020 15:17

Fix rebase conflict

4d27264

Fix prerendering

6cd14f5

Adjust tests

07c001d

Replace String.chop in generator

cafb984

icidasset commented Mar 28, 2020

View reviewed changes

dillonkearns added 3 commits March 28, 2020 13:02

Fix prefetching by checking if a route is prefetchable based on the h…

e10c062

…ow the URL resolves. Uses https://developer.mozilla.org/en-US/docs/Web/API/URL/URL.

Prevent base href from being set to unknown.

6649ce7

Use document.baseURI instead of looking up the base tag.

964f9af

Change formatting to include trailing slash for empty cleaned route.

9328cf2

dillonkearns approved these changes Mar 28, 2020

View reviewed changes

Merge branch 'master' into pr/73

d61d4a3

dillonkearns merged commit 3fbf618 into dillonkearns:master Mar 28, 2020

ianmackenzie mentioned this pull request Mar 29, 2020

Support sites hosted not at the top level #15

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make paths relative #73

Make paths relative #73

icidasset commented Feb 20, 2020 •

edited

Loading

cyberglot commented Feb 21, 2020

cyberglot commented Feb 21, 2020

icidasset commented Feb 21, 2020 •

edited

Loading

icidasset commented Feb 21, 2020

icidasset commented Feb 26, 2020

dillonkearns commented Mar 3, 2020

icidasset commented Mar 3, 2020

dillonkearns commented Mar 3, 2020

icidasset commented Mar 3, 2020

icidasset commented Mar 10, 2020

dillonkearns commented Mar 11, 2020

icidasset commented Mar 11, 2020

dillonkearns commented Mar 11, 2020

icidasset commented Mar 27, 2020

dillonkearns commented Mar 27, 2020

icidasset Mar 28, 2020 •

edited

Loading

dillonkearns Mar 28, 2020

dillonkearns commented Mar 28, 2020

icidasset commented Mar 28, 2020 •

edited

Loading

dillonkearns commented Mar 28, 2020

icidasset commented Mar 28, 2020 •

edited

Loading

dillonkearns commented Mar 28, 2020

Make paths relative #73

Make paths relative #73

Conversation

icidasset commented Feb 20, 2020 • edited Loading

How it works

Example

Notes

Issues

Testing

Other solutions

cyberglot commented Feb 21, 2020

cyberglot commented Feb 21, 2020

icidasset commented Feb 21, 2020 • edited Loading

icidasset commented Feb 21, 2020

icidasset commented Feb 26, 2020

dillonkearns commented Mar 3, 2020

icidasset commented Mar 3, 2020

dillonkearns commented Mar 3, 2020

icidasset commented Mar 3, 2020

icidasset commented Mar 10, 2020

dillonkearns commented Mar 11, 2020

icidasset commented Mar 11, 2020

dillonkearns commented Mar 11, 2020

icidasset commented Mar 27, 2020

dillonkearns commented Mar 27, 2020

icidasset Mar 28, 2020 • edited Loading

Choose a reason for hiding this comment

dillonkearns Mar 28, 2020

Choose a reason for hiding this comment

dillonkearns commented Mar 28, 2020

Link prefetching

unknown in base url

One small code change

Does the <base> tag need to update on page changes?

icidasset commented Mar 28, 2020 • edited Loading

dillonkearns commented Mar 28, 2020

icidasset commented Mar 28, 2020 • edited Loading

dillonkearns commented Mar 28, 2020

icidasset commented Feb 20, 2020 •

edited

Loading

icidasset commented Feb 21, 2020 •

edited

Loading

icidasset Mar 28, 2020 •

edited

Loading

`unknown` in base url

Does the `<base>` tag need to update on page changes?

icidasset commented Mar 28, 2020 •

edited

Loading

icidasset commented Mar 28, 2020 •

edited

Loading