More control over file path and URL for remote assets about repack

[adammruk]

Hi @lstoyanoff ! I'm happy that I'm not the only one having this case 😅
I agree, there are different usecases, technically different scale outputs a different content hash, but the general issue is to identify each asses, not necessary each scale. That's why I'm opting for more flexible approach, where you can decide it for yourself.

@jbroma any updates about that change?

[jbroma]

Hi @adammruk,

I love the proposal, this is definitely a functionality that should be there! I'm not sure about the final API, hear me out:

Remote assets functionality is something that very much resembles asset/resource from Webpack, so I was thinking of aligning the API to utilise that. Take a look here: https://webpack.js.org/guides/asset-modules/#custom-output-filename and also here: https://webpack.js.org/configuration/module/#rulegeneratorfilename since these options are related. This also allows use to re-use the templating functionality from Webpack.

WDYT? Since this is a breaking change, it would require a new major, we could also implement an experimental API using something similar (or exactly like that) to what you've provided.

[adammruk]

Hi @jbroma, nice to talk with you again :D

Ok that looks super promising, I love the idea of reusing webpack templating! It crossed my mind while working on that if it would be possible. I'm not insisting on the API, the example was more about explaining the problem.

So, if I understand it correctly, we could have something like this:

remote: {
  enabled: isRemoteAsset,
  assetModuleFilename: "[path]/[name]-[hash].[ext]" 
  publicPath: getRemoteAssetsPublicPath(platform),
},

So the "assetModuleFilename" field will be applied only to remote assets.
Why do you think this is a breaking change, since it will be a new field? It should be backwards compatibile, cause not specifying this field will just work the same as now. Od did I just misunderstood something 😅

[jbroma]

I was thinking even more than that: we could reuse the existing fields in webpack config without duplicating them - they only get applied to asset modules of type asset or asset/resource anyways, and these are not used atm on platforms like android and ios with Re.Pack.

We could go even a step further and override the asset/resource for android/ios platforms through createGenerator hook from Webpack (docs), so essentially running the that hook in RepackTargetPlugin.

[adammruk]

Sorry for such late response, I have been off for the whole week.
I'm not super familiar with all of those stuff 😅 But generally speaking this approach should solve our issue. Can you point me please some similar places in the code so I can take a look how this should be done?

[adammruk]

Hi @jbroma!

I have been playing around with it a bit, but I think I’m stuck. I tried to utilize assetModuleFilename in the webpack config, within the output field, but I think I’m missing a piece here. I think the main issue is that I don’t understand when and where the template passed to the assetModuleFilename field is actually transformed into the final path.

I feel like the logic should be inside Repack’s AssetsLoader because we are calling convertToRemoteAssets from there, so at that point, the final path should be available. Could you give me a few more hints on how to continue with that?

[jbroma]

Hi @adammruk, sorry for the the delayed response on my part this time!

there is no way to utilize assetModuleFilename right now, the assetLoader would need to become a replacement for asset generator (specified using type: asset/resource as an example, instead of use: '@callstack/repack/assetLoader'). Perhaps I went a little bit too far ahead in the concepts here 😅

Lets go back to the beginning, I think we should proceed with your original idea and just mark the API as unstable to leave some room for improvements and denote that explicitly. Perhaps a callback with some asset info like assetPath or assetData could prove useful for generating the desired filePath?

[adammruk]

That's fine:D
Ok, so it is indeed not that easy 😅 But anyway sounds like a nice final solution.
So if it's fine for you, I can move forward with the initial idea. I have been experimenting with the callback function, but the problem is that we have a validation here: https://github.com/callstack/repack/blob/main/packages/repack/src/webpack/loaders/assetsLoader/options.ts
and I cannot pass the function as a type for a field. I can't remember the original error but it was complaining that function type is not compatible with JSON Schema.

I'm thinking about the possibility to use webpack templating here, so we could pass something like this:

assetModuleFilename: "[path]/[name]-[hash].[ext]" 

to have at least a little bit of aligned API with webpack standards, but I'm not sure how to transform this template in the Repack's AssetsLoader to the "real" filename. WDYT?

[adammruk]

Ok I was able to find a solution, still testing it but initial checks looks promising, I updated my PR: https://github.com/adammruk/repack/pull/1/files
What are your thoughts about it?

[jbroma]

Hi @adammruk

For the function type, I think we can use instanceOf: 'Function' instead of type like in packages/repack/src/webpack/plugins/CodeSigningPlugin/config.ts (there it's used for RegExp).

I'm not sure about using interpolateName from loader-utils since we've recently removed loader-utils as a dependency of Re.Pack in #628 - loader-utils might not be compatible with rspack in V5. I took a quick look and it should be fine but that might change in the future.

The rest of that looks good to me 🚀, I guess we should open a PR a discuss implementation details there 👍

[adammruk]

Ok great, thanks for the hints! 🚀 I'm sitting on 3.4.0 in my project that's why I was tinkering with that version, was a bit easier to test for me. I'll switch now to some example project:)

{ instanceOf: 'Function'} is not throwing the error, so I'll try go with function approach. PR soon :D

[adammruk]

Hi @jbroma ! PR here: #651 - let me know what do you think and what can be improved :)

[lstoyanoff]

Hey! I was tinkering with hashes for remote assets as well and come up with the following hack 😅 It for sure can be improved further. Maybe it's not worth calculating the content hash for each asset scale (if any) or even the hash can be calculated for the file modification date instead.

@callstack+repack+3.7.0.patch

[jbroma]

Hi @lstoyanoff ! I'm happy that I'm not the only one having this case 😅 I agree, there are different usecases, technically different scale outputs a different content hash, but the general issue is to identify each asses, not necessary each scale. That's why I'm opting for more flexible approach, where you can decide it for yourself.

@jbroma any updates about that change?

feels like we should go with a flexible approach for now, we can always address things later if needed.
@adammruk I've reviewed the PR 👍