Jelajahi Sumber

fix: improve content-uri handling

The old implementation didn't handle absolute paths properly (#255), so
this is an attempt at solving that issue.

It turned out that the `urlJoin` utility-function was only used in a
single place, where it could be replaced with the URL constructor, so I
removed both the utility-function as well as its tests.

That left only a single call to `path.basename` remaining as the reason
to have the `path-browserify` dependency, so I replaced that as well and
removed the dependency.
Martin Schuhfuss 3 tahun lalu
induk
melakukan
7b75d9fb93
4 mengubah file dengan 3 tambahan dan 81 penghapusan
  1. 0 3
      package.json
  2. 3 4
      src/base/TilesRendererBase.js
  3. 0 33
      src/utilities/urlJoin.js
  4. 0 41
      test/urlJoin.test.js

+ 0 - 3
package.json

@@ -55,8 +55,5 @@
   },
   "peerDependencies": {
     "three": ">=0.123.0"
-  },
-  "dependencies": {
-    "path-browserify": "^1.0.1"
   }
 }

+ 3 - 4
src/base/TilesRendererBase.js

@@ -1,5 +1,3 @@
-import path from 'path-browserify';
-import { urlJoin } from '../utilities/urlJoin.js';
 import { getUrlExtension } from '../utilities/urlExtension.js';
 import { LRUCache } from '../utilities/LRUCache.js';
 import { PriorityQueue } from '../utilities/PriorityQueue.js';
@@ -192,7 +190,8 @@ export class TilesRendererBase {
 
 			if ( tile.content.uri ) {
 
-				tile.content.uri = urlJoin( tileSetDir, tile.content.uri );
+				// tile content uri has to be interpreted relative to the tileset.json
+				tile.content.uri = new URL( tile.content.uri, tileSetDir ).toString();
 
 			}
 
@@ -315,7 +314,7 @@ export class TilesRendererBase {
 					'asset.version is expected to be a string of "1.0" or "0.0"'
 				);
 
-				const basePath = path.dirname( url );
+				const basePath = url.replace( /\/[^\/]*\/?$/, '' );
 
 				traverseSet(
 					json.root,

+ 0 - 33
src/utilities/urlJoin.js

@@ -1,33 +0,0 @@
-import path from 'path-browserify';
-
-// Function that properly handles path resolution for parts that have
-// a protocol component like "http://".
-export function urlJoin( ...args ) {
-
-	const protocolRegex = /^[a-zA-Z]+:\/\//;
-	let lastRoot = - 1;
-	for ( let i = 0, l = args.length; i < l; i ++ ) {
-
-		if ( protocolRegex.test( args[ i ] ) ) {
-
-			lastRoot = i;
-
-		}
-
-	}
-
-	if ( lastRoot === - 1 ) {
-
-		return path.join( ...args ).replace( /\\/g, '/' );
-
-	} else {
-
-		const parts = lastRoot <= 0 ? args : args.slice( lastRoot );
-		const protocol = parts[ 0 ].match( protocolRegex )[ 0 ];
-		parts[ 0 ] = parts[ 0 ].substring( protocol.length );
-
-		return ( protocol + path.join( ...parts ) ).replace( /\\/g, '/' );
-
-	}
-
-}

+ 0 - 41
test/urlJoin.test.js

@@ -1,41 +0,0 @@
-import { urlJoin } from '../src/utilities/urlJoin.js';
-
-describe( 'urlJoin', () => {
-
-	it( 'should behave like path.join if no protocol is present', () => {
-
-		expect(
-			urlJoin( 'path', 'to', 'file.json' )
-		).toBe( 'path/to/file.json' );
-
-		expect(
-			urlJoin( 'path//', 'to/other/', 'file.json' )
-		).toBe( 'path/to/other/file.json' );
-
-	} );
-
-	it( 'should handle protocols correctly.', () => {
-
-		expect(
-			urlJoin( 'http://path', 'to', 'file.json' )
-		).toBe( 'http://path/to/file.json' );
-
-		expect(
-			urlJoin( 'http://path', 'http://path2', 'to', 'file.json' )
-		).toBe( 'http://path2/to/file.json' );
-
-		expect(
-			urlJoin( 'https://path', 'to', 'file.json' )
-		).toBe( 'https://path/to/file.json' );
-
-		expect(
-			urlJoin( 'ftp://path', 'to', 'file.json' )
-		).toBe( 'ftp://path/to/file.json' );
-
-		expect(
-			urlJoin( 'ftp://http://path', 'to', 'file.json' )
-		).toBe( 'ftp://http:/path/to/file.json' );
-
-	} );
-
-} );