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

feat(compiler-sfc): support transforming asset urls with full base url. #2477

Merged
merged 2 commits into from
Dec 4, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,5 +1,37 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`compiler sfc: transform asset url should allow for full base URLs, with paths 1`] = `
"import { createVNode as _createVNode, openBlock as _openBlock, createBlock as _createBlock } from \\"vue\\"

export function render(_ctx, _cache) {
return (_openBlock(), _createBlock(\\"img\\", { src: \\"http://localhost:3000/src/logo.png\\" }))
Copy link
Contributor Author

@joeldenning joeldenning Oct 23, 2020

Choose a reason for hiding this comment

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

Notice the fully qualified URL for the img src - this was previously not possible when using options.base, as far as I could tell

}"
`;

exports[`compiler sfc: transform asset url should allow for full base URLs, without paths 1`] = `
"import { createVNode as _createVNode, openBlock as _openBlock, createBlock as _createBlock } from \\"vue\\"

export function render(_ctx, _cache) {
return (_openBlock(), _createBlock(\\"img\\", { src: \\"http://localhost:3000/logo.png\\" }))
}"
`;

exports[`compiler sfc: transform asset url should allow for full base URLs, without port 1`] = `
"import { createVNode as _createVNode, openBlock as _openBlock, createBlock as _createBlock } from \\"vue\\"

export function render(_ctx, _cache) {
return (_openBlock(), _createBlock(\\"img\\", { src: \\"http://localhost/logo.png\\" }))
}"
`;

exports[`compiler sfc: transform asset url should allow for full base URLs, without protocol 1`] = `
"import { createVNode as _createVNode, openBlock as _openBlock, createBlock as _createBlock } from \\"vue\\"

export function render(_ctx, _cache) {
return (_openBlock(), _createBlock(\\"img\\", { src: \\"//localhost/logo.png\\" }))
}"
`;

exports[`compiler sfc: transform asset url support uri fragment 1`] = `
"import { createVNode as _createVNode, openBlock as _openBlock, createBlock as _createBlock } from \\"vue\\"
import _imports_0 from '@svg/file.svg'
Expand Down
32 changes: 32 additions & 0 deletions packages/compiler-sfc/__tests__/templateTransformAssetUrl.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -94,4 +94,36 @@ describe('compiler sfc: transform asset url', () => {
// should not remove it
expect(code).toMatch(`"xlink:href": "#myCircle"`)
})

test('should allow for full base URLs, with paths', () => {
const { code } = compileWithAssetUrls(`<img src="./logo.png" />`, {
base: 'http://localhost:3000/src/'
})

expect(code).toMatchSnapshot()
})

test('should allow for full base URLs, without paths', () => {
const { code } = compileWithAssetUrls(`<img src="./logo.png" />`, {
base: 'http://localhost:3000'
})

expect(code).toMatchSnapshot()
})

test('should allow for full base URLs, without port', () => {
const { code } = compileWithAssetUrls(`<img src="./logo.png" />`, {
base: 'http://localhost'
})

expect(code).toMatchSnapshot()
})

test('should allow for full base URLs, without protocol', () => {
const { code } = compileWithAssetUrls(`<img src="./logo.png" />`, {
base: '//localhost'
})

expect(code).toMatchSnapshot()
})
})
13 changes: 9 additions & 4 deletions packages/compiler-sfc/src/templateTransformAssetUrl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -120,12 +120,17 @@ export const transformAssetUrl: NodeTransform = (
attr.value.content[0] !== '@' &&
isRelativeUrl(attr.value.content)
) {
// Allow for full hostnames provided in options.base
const base = parseUrl(options.base)
const protocol = base.protocol || ''
const host = base.host ? protocol + '//' + base.host : ''
const basePath = base.path || '/'

// when packaged in the browser, path will be using the posix-
// only version provided by rollup-plugin-node-builtins.
attr.value.content = (path.posix || path).join(
options.base,
url.path + (url.hash || '')
)
attr.value.content =
host +
(path.posix || path).join(basePath, url.path + (url.hash || ''))
Copy link
Contributor Author

@joeldenning joeldenning Oct 23, 2020

Choose a reason for hiding this comment

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

path.join() does not work well with full URLs, which is why I'm prepending host here and only joining the path portion of the url.

// Notice how path.join() strips the second slash from http://
// This is the problem that is solved by this pull request.
path.join("http://localhost:3000/src/assets", "logo.png") === 'http:/localhost:3000/src/assets/logo.png')

}
return
}
Expand Down
2 changes: 1 addition & 1 deletion packages/compiler-sfc/src/templateUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,5 +35,5 @@ export function parseUrl(url: string): UrlWithStringQuery {
function parseUriParts(urlString: string): UrlWithStringQuery {
// A TypeError is thrown if urlString is not a string
// @see https://nodejs.org/api/url.html#url_url_parse_urlstring_parsequerystring_slashesdenotehost
return uriParse(isString(urlString) ? urlString : '')
return uriParse(isString(urlString) ? urlString : '', false, true)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The true here is necessary to correctly handle URLs that include hostname but not protocol, for example //localhost

}