Remove baseUrl if its just a slash

[VitaliyR]

Marked version: master

Description

• Fixes baseUrl with relative prefix not working as expected. #1466

Contributor

• Test(s) exist to ensure functionality and minimize regression (if no tests added, list tests covering this PR); or,
• no tests required for this PR.
• If submitting new feature, it has been documented in the appropriate places.

Committer

In most cases, this should be a different person than the contributor.

• Draft GitHub release notes have been updated.
• CI is green (no forced merge required).
• Merge PR

[UziTech]

I was just looking at the code and it looks like the issue is that baseUrl is supposed to be an absolute url.

marked/lib/marked.js

Lines 1435 to 1436 in 124c576

	} else if (href.charAt(0) === '/') {
	return base.replace(/(:\/*[^/]*)[\s\S]*/, '$1') + href;

I'm not sure if that was the intention when baseUrl was proposed.

@styfle Instead of creating a special case for baseUrl: '/' should we fix resolveUrl to work with a relative baseUrl?

We should create a test file like relative_urls.md with baseUrl: '/base/'.

[styfle]

The docs say baseUrl is a prefix, but the name URL makes me think it should be a complete URL like the <base> tag from HTML https://developer.mozilla.org/en-US/docs/Web/HTML/Element/base.

[UziTech]

So the ultimate questions is should we update resolveUrl to work with a relative baseUrl or update the docs to make it clear that baseUrl should be an absolute url?

I'm not exactly sure why baseUrl was added as an option. It looks like @joshbruce added it in e5b2998.

[VitaliyR]

I think typically baseUrl should be available - and works just as path.join, not concatenating. I mean in case of href of link/img in the parser -> path.join(baseUrl || '', href)

[UziTech]

I think it works more like path.resolve(base, href).

• Any href starting with / is considered absolute relative to the domain.
• Any href starting with // will replace the domain but not the protocol.
• Any href starting with .*:// is considered absolute and the baseUrl will be ignored.

[styfle]

I'm starting to think baseUrl was a bad choice. Instead, we should have a callback to allow the user to specify what to do with urls.

Just like the highlight function, we could have a url function.

Or is this a job for the Renderer?

[UziTech]

I think it should be a job for the renderer.

I'm not exactly sure why this option was added in the first place. It was added by @joshbruce in e5b2998 without a good explanation

[styfle]

@joshbruce Can we remove this baseUrl option in favor of overriding the Renderer?

[VitaliyR]

@styfle remove it at all? So is there a way to prepend some url to all urls which are transformed by marked?

Or it will be via overriding Renderer.image && link?

[UziTech]

I don't think we will remove it since there are definitely people using it but this is one option we will be deprecating in the future and probably have an extension that overrides the renderer.

[UziTech]

I feel like in the meantime we should make baseUrl work with relative urls. @VitaliyR do you want to change this PR to handle that or should I create a new PR?

[VitaliyR]

@UziTech
What exactly do you mean? Because now:

baseUrl: 'http://example.com/root'
url: /pic/img.jpg

result: http://example.com/pic/img.jpg (without root in the path)

So what should we do in this case? Because we might break something

[UziTech]

the baseUrl needs to end with a slash

marked("![](pic/img.jpg)", {baseUrl: "http://example.com/root/"});
// <p><img src="https://example.com/root/pic/img.jpg" alt=""></p>