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

fix(scully): handle # and ? in urls properly #516

Merged
merged 1 commit into from
Apr 28, 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
Expand Up @@ -12,6 +12,7 @@ import {filter, take, tap} from 'rxjs/operators';
import {ScullyRoutesService} from '../route-service/scully-routes.service';
import {fetchHttp} from '../utils/fetchHttp';
import {findComments} from '../utils/findComments';
import {basePathOnly} from '../utils/basePathOnly';

interface ScullyContent {
html: string;
Expand All @@ -25,7 +26,6 @@ declare global {
/** this is needed, because otherwise the CLI borks while building */
const scullyBegin = '<!--scullyContent-begin-->';
const scullyEnd = '<!--scullyContent-end-->';
const dropEndingSlash = (str: string) => (str.endsWith('/') ? str.slice(0, -1) : str);

@Component({
// tslint:disable-next-line: component-selector
Expand Down Expand Up @@ -77,7 +77,7 @@ export class ScullyContentComponent implements OnDestroy, OnInit {
* Will fetch the content from sibling links with xmlHTTPrequest
*/
private async handlePage() {
const curPage = dropEndingSlash(location.href);
const curPage = basePathOnly(location.href);
if (this.lastHandled === curPage) {
/**
* Due to the fix we needed for #311
Expand Down Expand Up @@ -111,7 +111,7 @@ export class ScullyContentComponent implements OnDestroy, OnInit {
.catch(e => {
if (isDevMode()) {
const uri = new URL(location.href);
const url = `http://localhost:1668/${dropEndingSlash(uri.pathname)}/index.html`;
const url = `http://localhost:1668/${basePathOnly(uri.pathname)}/index.html`;
return fetchHttp(url, 'text');
} else {
throw new Error(e);
Expand Down Expand Up @@ -156,11 +156,11 @@ export class ScullyContentComponent implements OnDestroy, OnInit {
*/
async upgradeToRoutelink(elm: HTMLElement) {
const routes = await this.routes;
const lnk = dropEndingSlash(elm.getAttribute('href').toLowerCase());
const route = routes.find(r => dropEndingSlash(r.route.toLowerCase()) === lnk);
const lnk = basePathOnly(elm.getAttribute('href').toLowerCase());
const route = routes.find(r => basePathOnly(r.route.toLowerCase()) === lnk);

/** only upgrade routes known by scully. */
if (lnk && route) {
if (lnk && route && !lnk.startsWith('#')) {
elm.onclick = async (ev: MouseEvent) => {
const splitRoute = route.route.split(`/`);
const curSplit = location.pathname.split('/');
Expand All @@ -176,8 +176,11 @@ export class ScullyContentComponent implements OnDestroy, OnInit {
return;
}

/** check for the same route with different "data", and NOT a level higher (length) */
if (curSplit.every((part, i) => splitRoute[i] === part) && splitRoute.length > curSplit.length) {
/** check for the same route with different "data", and NOT a 1 level higher (length) */
if (
curSplit.every((part, i) => splitRoute[i] === part) &&
splitRoute.length !== curSplit.length + 1
) {
setTimeout(() => this.replaceContent(), 10); // a small delay, so we are sure the angular parts in the page are settled enough
}
};
Expand Down
14 changes: 14 additions & 0 deletions ng-projects/scullyio/ng-lib/src/lib/utils/basePathOnly.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
/**
* Take a string, preferably resembling an URL, take out the search params, the anchors, and the ending slash
* @param str
*/
export const basePathOnly = (str: string): string => {
if (str.includes('#')) {
str = str.split('#')[0];
}
if (str.includes('?')) {
str = str.split('?')[0];
}
const cleanedUpVersion = str.endsWith('/') ? str.slice(0, -1) : str;
return cleanedUpVersion;
};
20 changes: 20 additions & 0 deletions scully/routerPlugins/addOptionalRoutesPlugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,26 @@ async function routePluginHandler(route: string): Promise<HandledRoute[]> {
)} is invalid.`
);
}
if (handledRoute.route.includes('?')) {
const updatedRoute = handledRoute.route.split('?')[0];
logWarn(
`The route "${yellow(
handledRoute.route
)}" contains a search param, this will be ignored during rendering. it will be truncated to:
"${yellow(updatedRoute)}"`
);
handledRoute.route = updatedRoute;
}
if (handledRoute.route.includes('#')) {
const updatedRoute = handledRoute.route.split('#')[0];
logWarn(
`The route "${yellow(
handledRoute.route
)}" contains a hash(#), this will be ignored during rendering. it will be truncated to:
"${yellow(updatedRoute)}"`
);
handledRoute.route = updatedRoute;
}
});
return generatedRoutes;
}
Expand Down
1 change: 1 addition & 0 deletions tests/cypress/integration/sampleBlog.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ context('check first integration test', () => {
cy.get('ul>li>a')
.contains('/user')
.click()
.wait(25) // give the async fetch a bit of time to complete
.get('a')
.contains('Leanne Graham')
.click()
Expand Down