Skip to content

Commit 2f84d3b

Browse files
authored
fix: add relative path to component links in static builds (#1062)
* add relative path to component links in static builds In a static build the links to components now begin with `./` incase a user has set a handle with a `:` in it. Having a `:` in your component handle would produce a static url like: `href="foo:btn.html"` The browser tries to interpret this as an app link, which gives you a `Failed to launch 'foo:btn' because the scheme does not have a registered handler` error message in the browser developer console and stops page navigation. Having a relative path current folder `./`, so producing `href="./foo:btn.html"` stops the browser interpreting as an app link. For server builds, this doesn't apply, the existing paths are used as before. Why use `:` in a component handle? Components may naturally have `:` in their name, such as `ratio-box--16:9` or a user maybe name spacing components in a multisite Fractal library, where components may have the same name but live in different sites: ``` {% include '@foo:btn' with data %} // outputs `btn` from site `foo`, "handle": "foo:btn" {% include '@bar:btn' with data %} // outputs `btn` from site `bar`, "handle": "bar:btn" ``` * add `relativeToCurrentFolder` option to utils `relUrlPath` so we can optionally choose to have a relative path from the current folder or not * use `relativeToCurrentFolder` option of `relUrlPath` in path helpers * update tests to test for `relativeToCurrentFolder` usage * update snapshots for build tests for relative paths from current dir * add `relativeToCurrentFolder` to fractal config and revert various path/adapters to previous state, as they all read `app.web.get('builder.urls')` and so get the new `builder.urls.relativeToCurrentFolder` option * don't add `./` to output url of `relUrlPath` if starts with `.` Tests updated to match * add utils `returns correct relative path to file with extension` test variations Co-authored-by: Mike Byrne <mike@area17.com>
1 parent 90192d0 commit 2f84d3b

File tree

7 files changed

+68
-9
lines changed

7 files changed

+68
-9
lines changed

examples/handlebars/components/path/__snapshots__/path.spec.js.snap

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
// Jest Snapshot v1, https://goo.gl/fbAQLP
22

33
exports[`path converts absolute path to relative for builder 1`] = `
4-
"some-path.html
4+
"./some-path.html
55
"
66
`;
77

examples/nunjucks/components/path/__snapshots__/path.spec.js.snap

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
// Jest Snapshot v1, https://goo.gl/fbAQLP
22

33
exports[`path converts absolute path to relative for builder 1`] = `
4-
"some-path.html
4+
"./some-path.html
55
"
66
`;
77

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
// Jest Snapshot v1, https://goo.gl/fbAQLP
22

3-
exports[`path converts absolute path to relative for builder 1`] = `"some-path.html"`;
3+
exports[`path converts absolute path to relative for builder 1`] = `"./some-path.html"`;
44

55
exports[`path renders original path for server 1`] = `"/some-path"`;

examples/twig/components/path/__snapshots__/path.spec.js.snap

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
// Jest Snapshot v1, https://goo.gl/fbAQLP
22

33
exports[`path converts absolute path to relative for builder 1`] = `
4-
"some-path.html
4+
"./some-path.html
55
"
66
`;
77

packages/core/src/utils.js

Lines changed: 19 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -146,14 +146,28 @@ module.exports = {
146146
fromPath = getStaticPagePath(fromPath).replace(/\\/g, '/');
147147
toPath = ('/' + _.trim(Path.extname(toPath) ? toPath : getStaticPagePath(toPath), '/')).replace(/\\/g, '/');
148148

149+
let outputPath;
150+
149151
if (toPath == '/') {
150-
return Path.relative(fromPath, toPath).replace(/\\/g, '/');
152+
outputPath = Path.relative(fromPath, toPath).replace(/\\/g, '/');
153+
} else {
154+
outputPath = Path.relative(fromPath, toPath)
155+
.replace(/\\/g, '/')
156+
.replace(/^\.\.\//, '')
157+
.replace('.PLACEHOLDER', ext);
151158
}
152159

153-
return Path.relative(fromPath, toPath)
154-
.replace(/\\/g, '/')
155-
.replace(/^\.\.\//, '')
156-
.replace('.PLACEHOLDER', ext);
160+
// for static builds, we want urls relative to the current directory
161+
// eg: ./btn.html
162+
// so that any use of `:` within a handle generates links as `./foo:btn.html`
163+
// and not just `foo:btn.html` as browsers may try to interpret the `:`
164+
// as an application link, like `<a href="twitter://user?screen_name=clearleft">open</a>``
165+
// which would open the twitter app.
166+
// links like `foo:btn.html` the browser tries to open an app "foo", fails and emits:
167+
// `Failed to launch 'foo:btn' because the scheme does not have a registered handler`
168+
// and halts navigation. Adding the `./`, giving it a relative path to the current folder
169+
// stops the browser interpreting the link as an app link, behaves as a normal link
170+
return opts && opts.relativeToCurrentFolder && !outputPath.startsWith('.') ? `./${outputPath}` : outputPath;
157171

158172
function getStaticPagePath(url) {
159173
if (url == '/') {

packages/core/test/utils.spec.js

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -302,6 +302,50 @@ describe('Utils', () => {
302302
it('returns correct relative path to file with extension', () => {
303303
expect(utils.relUrlPath('../to/image.png', '/path/b', opts)).toEqual('../to/image.png');
304304
});
305+
306+
// for static builds
307+
const opts2 = {
308+
ext: '.html',
309+
relativeToCurrentFolder: true,
310+
};
311+
const opts3 = {
312+
relativeToCurrentFolder: true,
313+
};
314+
it('returns correct relative path from current directory for same directory', () => {
315+
expect(utils.relUrlPath('/path/to/a', '/path/to/b', opts2)).toEqual('./a.html');
316+
});
317+
318+
it('returns correct relative path from the current directory for parent directory', () => {
319+
expect(utils.relUrlPath('/path/to/a', '/path/b', opts2)).toEqual('./to/a.html');
320+
});
321+
322+
it('returns toPath if it is full url even if `relativeToCurrentFolder` is true', () => {
323+
expect(utils.relUrlPath('https://fractal.build', '/path/b', opts2)).toEqual('https://fractal.build');
324+
});
325+
326+
it('returns toPath with extension if it is already a relative path from current directory', () => {
327+
expect(utils.relUrlPath('./path/to/a', '/path/b', opts2)).toEqual('./path/to/a.html');
328+
});
329+
330+
it('returns toPath without extension if it is already a relative path from current directory', () => {
331+
expect(utils.relUrlPath('./path/to/a', '/path/b', opts3)).toEqual('./path/to/a');
332+
});
333+
334+
it('returns correct path to root from current directory', () => {
335+
expect(utils.relUrlPath('/', '/path/b', opts3)).toEqual('../..');
336+
});
337+
338+
it('returns correct path to root with extension from current directory', () => {
339+
expect(utils.relUrlPath('/', '/path/b', opts2)).toEqual('../index.html');
340+
});
341+
342+
it('returns correct path to file with extension from current directory', () => {
343+
expect(utils.relUrlPath('/path/to/image.png', '/path/b', opts2)).toEqual('./to/image.png');
344+
});
345+
346+
it('returns correct relative path to file with extension from current directory', () => {
347+
expect(utils.relUrlPath('../to/image.png', '/path/b', opts2)).toEqual('../to/image.png');
348+
});
305349
});
306350
});
307351

packages/fractal/config.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,7 @@ module.exports = {
116116
ext: '.html',
117117
urls: {
118118
ext: '.html',
119+
relativeToCurrentFolder: true,
119120
},
120121
static: {
121122
ignored: [],

0 commit comments

Comments
 (0)