-
Notifications
You must be signed in to change notification settings - Fork 24
Add support for cache busting assets - sync version #45
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -408,6 +408,29 @@ Cartero.prototype.processMains = function( callback ) { | |
|
|
||
| _this.packagePathsToIds[ newPackage.path ] = newPackage.id; | ||
|
|
||
| // calculates the shasum for the assets | ||
| _this.assetMap = _this.assetMap || {}; | ||
| async.each( assetTypesToWriteToDisk, function( thisAssetType, nextAssetType ) { | ||
| // We dont need to process assets that are in assetTypesToConcatenate | ||
| if ( _this.assetTypesToConcatenate.indexOf( thisAssetType ) !== -1 ) return nextAssetType(); | ||
|
|
||
| async.each( newPackage.assetsByType[ thisAssetType ], function( thisAsset, nextAsset ) { | ||
|
|
||
| var fileContent = fs.readFileSync( thisAsset.srcPath, 'utf-8' ); | ||
| var shasum = crypto.createHash( 'sha1' ); | ||
|
|
||
| shasum.update(fileContent); | ||
|
|
||
| var fileShasum =shasum.digest('hex'); | ||
| var fileName = path.relative( newPackage.path, thisAsset.srcPath ); | ||
| var fileExt = path.extname( fileName ); | ||
| var newFileName = path.basename( fileName, fileExt ) + '_' + fileShasum + fileExt; | ||
|
|
||
| // save the old name and new name for later use with the transforms | ||
| _this.assetMap[ thisAsset.srcPath ] = newFileName; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I like it! |
||
| }); | ||
| }); | ||
|
|
||
| newPackage.addTransform( replaceStringTransform, { | ||
| find : /url\(\s*[\"\']?([^)\'\"]+)\s*[\"\']?\s*\)/g, | ||
| replace : function( file, match, theUrl ) { | ||
|
|
@@ -417,12 +440,25 @@ Cartero.prototype.processMains = function( callback ) { | |
| if( theUrl.charAt( 0 ) === '/' ) return match; | ||
| if( theUrl.indexOf( 'data:' ) === 0 ) return match; // data url, don't mess with this | ||
|
|
||
| var absoluteAssetPath = path.resolve( path.dirname( file ), theUrl ); | ||
| var newAssetName = _this.assetMap[ absoluteAssetPath ]; | ||
|
|
||
| if ( newAssetName ) { | ||
| // replace the url with the new name | ||
| theUrl = path.join( path.dirname( theUrl ), newAssetName ); | ||
| } else { | ||
| // this happen when we have packages that have assets references that are not specified | ||
| // in the image tag in package.json. It happens in modules like jqueryui | ||
| log.warn( file, 'Asset reference ' + theUrl + ' cannot be resolved.' ); | ||
| } | ||
|
|
||
| var cssFilePathRelativeToPackageDir = path.relative( newPackage.path, file ); | ||
| var cssFileDirPathRelativeToPackageDir = path.dirname( '/' + cssFilePathRelativeToPackageDir ); | ||
| if( cssFileDirPathRelativeToPackageDir !== '/' ) cssFileDirPathRelativeToPackageDir += '/'; | ||
|
|
||
| // urls in css files are relative to the css file itself | ||
| var absUrl = url.resolve( cssFileDirPathRelativeToPackageDir, theUrl ); | ||
|
|
||
| absUrl = _this.outputDirUrl + newPackage.id + absUrl; | ||
|
|
||
| return 'url( \'' + absUrl + '\' )'; | ||
|
|
@@ -431,7 +467,8 @@ Cartero.prototype.processMains = function( callback ) { | |
|
|
||
| newPackage.addTransform( assetUrlTransform, { | ||
| packagePathsToIds : _this.packagePathsToIds, | ||
| outputDirUrl : _this.outputDirUrl | ||
| outputDirUrl : _this.outputDirUrl, | ||
| assetMap: _this.assetMap | ||
| }, 'style' ); | ||
|
|
||
| _this.writeIndividualAssetsToDisk( newPackage, assetTypesToWriteToDisk, function( err ) { | ||
|
|
@@ -515,7 +552,8 @@ Cartero.prototype.copyTempBundleToFinalDestination = function( tempBundlePath, a | |
| var postProcessorsToApply = _.clone( _this.postProcessors ); | ||
| if( assetType === 'script' ) postProcessorsToApply.unshift( function( file ) { return assetUrlTransform( file, { | ||
| packagePathsToIds : _this.packagePathsToIds, | ||
| outputDirUrl : _this.outputDirUrl | ||
| outputDirUrl : _this.outputDirUrl, | ||
| assetMap: _this.assetMap | ||
| } ); } ); | ||
|
|
||
| if( postProcessorsToApply.length !== 0 ) { | ||
|
|
@@ -683,17 +721,17 @@ Cartero.prototype.resolvePostProcessors = function( postProcessorNames, callback | |
|
|
||
| Cartero.prototype.writeIndividualAssetsToDisk = function( thePackage, assetTypesToWriteToDisk, callback ) { | ||
| var _this = this; | ||
| var pathsOfWrittenAssets = []; | ||
| var outputDirectoryPath = this.getPackageOutputDirectory( thePackage ); | ||
|
|
||
| assetTypesToWriteToDisk = _.intersection( assetTypesToWriteToDisk, Object.keys( thePackage.assetsByType ) ); | ||
|
|
||
| async.each( assetTypesToWriteToDisk, function( thisAssetType, nextAssetType ) { | ||
| async.each( thePackage.assetsByType[ thisAssetType ], function( thisAsset, nextAsset ) { | ||
| var thisAssetDstPath = path.join( outputDirectoryPath, path.relative( thePackage.path, thisAsset.srcPath ) ); | ||
| if( thisAssetType === 'style' ) thisAssetDstPath = renameFileExtension( thisAssetDstPath, '.css' ); | ||
|
|
||
| pathsOfWrittenAssets.push( thisAssetDstPath ); | ||
| var thisAssetDstPath = path.relative( thePackage.path, thisAsset.srcPath ); | ||
| thisAssetDstPath = path.join( outputDirectoryPath, path.dirname( thisAssetDstPath ), _this.assetMap[ thisAsset.srcPath ] ); | ||
|
|
||
| if( thisAssetType === 'style' ) thisAssetDstPath = renameFileExtension( thisAssetDstPath, '.css' ); | ||
|
|
||
| thisAsset.writeToDisk( thisAssetDstPath, function( err ) { | ||
| if( err ) return nextAsset( err ); | ||
|
|
@@ -760,7 +798,8 @@ Cartero.prototype.writeMetaDataFile = function( callback ) { | |
| var metaData = JSON.stringify( { | ||
| formatVersion : 2, | ||
| packageMap : packageMap, | ||
| entryPointMap : entryPointMap | ||
| entryPointMap : entryPointMap, | ||
| assetMap: _this.assetMap | ||
| }, null, 4 ); | ||
|
|
||
| fs.writeFile( metaDataFilePath, metaData, function( err ) { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| assets/ |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
| { | ||
| "view" : "page1.jade", | ||
| "main" : "page1.js", | ||
| "style" : "*.css", | ||
| "image": "img/robot.png" | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| body { | ||
| background : blue url('./img/robot.png'); | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| console.log( 'hellooo dave' ); | ||
|
|
||
| var robotPngPath = '##asset_url( "./img/robot.png" )'; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -148,13 +148,13 @@ test( 'example3', function( t ) { | |
| page1JsBundle = path.join( outputDirPath, parcelIdsByPath[ 'page1' ], page1JsBundle ); | ||
|
|
||
| var page1JsContents = fs.readFileSync( page1JsBundle, 'utf8' ); | ||
| t.ok( page1JsContents.indexOf( '/' + commonJsPackageId + '/robot.png' ) !== -1, '##asset_url resolved' ); | ||
| t.ok( page1JsContents.indexOf( '/' + commonJsPackageId + '/robot_9018f21e83ce46f3ea2e3b73e5d75ece75407df7.png' ) !== -1, '##asset_url resolved' ); | ||
|
|
||
| var page1CssBundle = _.find( page1PackageFiles, function( thisFile ) { return path.extname( thisFile ) === '.css'; } ); | ||
| page1CssBundle = path.join( outputDirPath, parcelIdsByPath[ 'page1' ], page1CssBundle ); | ||
|
|
||
| var page1CssContents = fs.readFileSync( page1CssBundle, 'utf8' ); | ||
| t.ok( page1CssContents.indexOf( '/' + commonJsPackageId + '/robot.png' ) !== -1, 'relative css url resolved' ); | ||
| t.ok( page1CssContents.indexOf( '/' + commonJsPackageId + '/robot_9018f21e83ce46f3ea2e3b73e5d75ece75407df7.png' ) !== -1, 'relative css url resolved' ); | ||
| t.ok( page1CssContents.indexOf( 'background : blue' ) !== -1, 'page 1 has correct background color' ); | ||
|
|
||
| var page2CssBundle = _.find( page2PackageFiles, function( thisFile ) { return path.extname( thisFile ) === '.css'; } ); | ||
|
|
@@ -165,9 +165,57 @@ test( 'example3', function( t ) { | |
|
|
||
| t.ok( _.contains( | ||
| fs.readdirSync( path.join( outputDirPath, commonJsPackageId ) ).sort(), | ||
| 'robot.png' | ||
| 'robot_9018f21e83ce46f3ea2e3b73e5d75ece75407df7.png' | ||
| ), 'robot.png in common package' ); | ||
| } ); | ||
| } ); | ||
|
|
||
|
|
||
| test( 'example4', function( t ) { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. thx for these extra tests!! |
||
| t.plan( 5 ); | ||
|
|
||
| var viewDirPath = path.join( __dirname, 'example4/views' ); | ||
| var outputDirPath = path.join( __dirname, 'example4/static/assets' ); | ||
| var packageId; | ||
| var packageMap = {}; | ||
|
|
||
| var c = cartero( viewDirPath, outputDirPath, {} ); | ||
|
|
||
| c.on( 'packageCreated', function( newPackage ) { | ||
| if( newPackage.isParcel ) { | ||
| packageId = newPackage.id; | ||
| } | ||
|
|
||
| packageMap[ newPackage.path ] = newPackage.id; | ||
| } ); | ||
|
|
||
| c.on( 'done', function() { | ||
| // Cannot determine the exact name o the files due to the fact that we calculate the shasum after some transformation of the code | ||
| // for more info check: https://github.com/rotundasoftware/cartero/pull/45 | ||
| var assets = require( path.join( outputDirPath, packageId, 'assets.json' ) ); | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @dgbeck that's how I fixed the test
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nice! |
||
| var cssFile = path.relative(packageId, assets.style[ 0 ]); | ||
| var jsFile = path.relative(packageId, assets.script[ 0 ]); | ||
| var imgDir = 'img'; | ||
| var imgFile = 'robot_9018f21e83ce46f3ea2e3b73e5d75ece75407df7.png'; | ||
|
|
||
| t.ok( fs.existsSync( path.join( outputDirPath, packageId, imgDir, imgFile ) ), 'robot.png created with the new name' ); | ||
|
|
||
| t.deepEqual( | ||
| fs.readdirSync( path.join( outputDirPath, packageId ) ).sort(), | ||
| [ 'assets.json', cssFile, jsFile, imgDir ].sort() | ||
| ); | ||
|
|
||
| t.deepEqual( | ||
| fs.readdirSync( path.join( outputDirPath, packageId, 'img') ), | ||
| [ imgFile ] | ||
| ); | ||
|
|
||
| // Test content of js file and check #asset_url | ||
| var jsContent = fs.readFileSync( path.join( outputDirPath, packageId, jsFile ), 'utf8' ); | ||
| t.notEqual( jsContent.indexOf( 'var robotPngPath = \'/' + [ packageId, imgDir, imgFile ].join( '/' ) + '\';' ), -1 ); | ||
|
|
||
| t.equal( fs.readFileSync( path.join( outputDirPath, packageId, cssFile ), 'utf8' ), | ||
| 'body {\n\tbackground : blue url( \'/' + [ packageId, imgDir, imgFile ].join( '/' ) + '\' );\n}' ); | ||
|
|
||
| } ); | ||
| } ); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -22,11 +22,16 @@ module.exports = function( file, options ) { | |
| var url = pathMapper( assetSrcAbsPath, function( srcDir ) { | ||
| return options.packagePathsToIds[ srcDir ] ? '/' + options.packagePathsToIds[ srcDir ] : null; // return val of outputDirPath needs to be absolute path | ||
| } ); | ||
|
|
||
| // all assets urls should be different than their paths.. otherwise we have a problem | ||
| if( url === assetSrcAbsPath ) | ||
| return _this.emit( 'error', new Error( 'The file "' + assetSrcAbsPath + '" referenced from ##asset_url( "' + assetSrcPath + '" ) in file "' + file + '" is not an asset.' ) ); | ||
|
|
||
| var filename = path.basename(assetSrcAbsPath); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this renaming seems to me outside of the intended scope of pathmapper, so I think this is in fact the best place for it, but the replace approach would break if filename occurs at unexpected locations in url. We should just figure out the last portion of url (whatever comes after the last "/") and replace that.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you please give me an example in which it will break? I'm not sure if I follow you.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. suppose filename == |
||
| var newFilename = options.assetMap[assetSrcAbsPath]; | ||
|
|
||
| url = path.join( path.dirname( url ), newFilename); | ||
|
|
||
| if( options.outputDirUrl ) { | ||
| var baseUrl = options.outputDirUrl; | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thx for following spacing conventions!