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

Dynamic assetPrefix via next runtime configuration utils #44

Merged

Conversation

peter-jozsa
Copy link
Contributor

@peter-jozsa peter-jozsa commented Aug 6, 2020

Aims to implement support for runtime configurable asset prefixes in next-images package which was previously mentioned in #12

@peter-jozsa
Copy link
Contributor Author

bump

Comment on lines +3 to +5
publicRuntimeConfig: Object.assign({}, nextConfig.publicRuntimeConfig, {
nextImagesAssetPrefix: nextConfig.assetPrefix
}),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't we move it to line 8? There we are initializing all configuration options.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately we can't move it from here because webpack part of next config is only used during build time. If you are in production "serve mode" aka next start webpack method wouldn't be called so root publicRuntimeConfig would not be touched.

index.js Outdated Show resolved Hide resolved
@@ -1,5 +1,8 @@
module.exports = (nextConfig = {}) => {
return Object.assign({}, nextConfig, {
publicRuntimeConfig: Object.assign({}, nextConfig.publicRuntimeConfig, {
nextImagesAssetPrefix: nextConfig.assetPrefix
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the difference between assetPrefix and nextImagesAssetPrefix ?

Copy link
Contributor Author

@peter-jozsa peter-jozsa Sep 24, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nextConfig.assetPrefix is a runtime configuration value which can be used to set asset prefix of js bundles and other asset files. (https://nextjs.org/docs/api-reference/next.config.js/cdn-support-with-asset-prefix) According to official next.js docs it is only used internally by next and it is not accessible. That's why we need to expose it explicitly in publicRuntimeConfig so we can access it confidently on server side and client side too.

Since users could define their own public runtime configuration I thought we shouldn't call it simply "assetPrefix" because maybe we override a user defined config variable. So I ended up using a "namespaced" variable name.

@arefaslani
Copy link
Member

@peter-jozsa Could you also update the readme please so that we know how to use the new feature?

@peter-jozsa peter-jozsa force-pushed the feature/dynamic-asset-prefix branch from 571be1d to e7c1eda Compare September 24, 2020 08:37
@peter-jozsa
Copy link
Contributor Author

As I checked availability of runtime configuration feature I saw it is only available starting from Next.js v6, is that a problem? Maybe this dynamic asset prefix feature should be opt-in since it will change transpiled image paths for everyone, what do you think?

@arefaslani
Copy link
Member

As I checked availability of runtime configuration feature I saw it is only available starting from Next.js v6, is that a problem? Maybe this dynamic asset prefix feature should be opt-in since it will change transpiled image paths for everyone, what do you think?

@peter-jozsa Let me check that after working hours. Will come back to you soon.

index.js Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants