Added support for custom asset paths and mapping

[alexindigo]

This PR pulls generation logic for external assets into a single function. And adds mapping support to allow next.js apps to be used with S3-like CDNs. Also it allows to override that function with custom logic to have more complex asset deploy pipeline (like per asset versioning or content hashing).

Added two examples:

custom-asset-map – shows how simple mapping could solve S3-like CDN issues (#2053, #2134)

custom-asset-path – shows how to use custom getAssetPath function in prod setup.

All my changes are grouped into this commit – e69e339

Second commit fixes styling issues that somehow leaked into the upstream branch and prevented tests from running

/cc @mtoso @dntrkv @rauchg

[JeremyJonas]

Thanks @alexindigo, Solid solution to the problem! I just started looking into how to do exactly this, you saved me tons of time!
That indeed should resolve #2053 and enable S3 CDN support of next build files for my use case. Your implementation is flexible enough to support other use cases as well.

[alexindigo]

Thanks @JeremyJonas :) We've hit the same issue when it was time to grow up out of dev/demo mode to the real thing :) When it's landed, I'll be playing with custom getAssetPath, to have each page's bundle content hashed. We'll see how it goes. :)

[timneutkens]

@alexindigo fixed the linting so that the PR only includes files for your changes 👍

[alexindigo]

Thank you @timneutkens :)
Looks like it broke the build though :)
Do you want me to file that cleaning commit as a separate PR?

[alexindigo]

There is another file to clean up: with-apollo-redux / components / PostList.js

[timneutkens]

@alexindigo it's actually failing on the integration tests because of a chromedriver timeout. The linting went through fine 😄 I'll check it out 👍

Edit: tests pass fine now 👍

[alexindigo]

Cool, thanks a lot. :) I’ll rebase when I get to the computer.

[alexindigo]

Rebased with the latest canary, so it's just my commit there. Let's see how it goes.

[alexindigo]

Btw @timneutkens what's the process for landing pull requests? Thank you.

[alexindigo]

Actually, canary itself is broken (at least in Travis) – https://travis-ci.org/zeit/next.js/builds/307099631

[alexindigo]

Yeah, those free CI runners all over the place with something more complex than simple unit tests :) #3338

[rauchg]

This looks great! Would it be possible to implement cdnPrefix as this? And if so, we should warn if both are supplied I think

[alexindigo]

@rauchg After I dug deeper into the code, I couldn’t find user-friendly way to supply custom cdnPrefix function (since it’s used both on the client and the server, and needed to be supplied after the build step) so I opted for stateless default function that could be overwritten with webpack) and assetMap that could be injected at runtime. And simple assetMap is enough in most cases (as you can see from the linked issues) and for hardcore things supplying custom getAssetPath function is not a big deal.
So cdnPrefix as a string makes more sense, especially considering isomorphic nature of the project.

[arunoda]

I think we've already fix this problem in our static export functionality. There's no need to supply a custom map.
We just need to apply that code base to all.

Here's an example: https://zeit-docs-dmmiqiafyj.now.sh/docs
(check the view source to pages loads from index.js.

I wrote this few months back, need to findout how I did this :)

[alexindigo]

@arunoda Do you mind to elaborate on what was fixed by static export functionality?

This PR solves couple issues:

1. Allowing nextjs apps to work with S3-like CDNs, where by default nextjs is requesting assets like cdn/_next/<build_id>/page/<pagename> – no extension, and static export outputs it like page/<pagename>/index.js – which requires special logic on the server to resolve former into latter.

2. Allowing to have custom request path, instead of cdn/_next/<build_id>/page/<pagename>, I can have something like cdn/<my-custom-version>/<my-custom-hash>/<pagename-related-filename>.<custom-extenision>.

[arunoda]

@alexindigo

To fix the first issue we don't need specific map. We need to append index.js for all the assets. We had to do that for our static export feature (which supports S3 and many others). So, it's matter of getting that feature to all.

For the second problem, I rather like to see a core solution where we omit the buildId completely but keep the sha-hash for that file. And mixing that with our assetPrefix option, you can achieve what you wanted.

Even though, this might fixes the problem in short term, I don't think this is best solution.
(Because, we build Next.js to getting away from a bulky configurations. These solution move us into that direction)

[alexindigo]

@arunoda Do you mind to provide link to a PR or chunk of code where it solved? I'm not sure we're talking about the same thing. It's definitely not solved in the canary branch, as you can see in the red parts of my PR.

For the second problem, I rather like to see a core solution where we omit the buildId completely but keep the sha-hash for that file.

It only solves one of the issues, but doesn't provide enough flexibility for developers to make it work for their deployment process. Like if they need release version instead of content hash of a file. Or they need to use commit hash for all their assets. Ability to inject custom logic for this kind of interaction between the app and the outside world is important for many teams.

Even though, this might fixes the problem in short term, I don't think this is best solution.
(Because, we build Next.js to getting away from a bulky configurations. These solution move us into that direction)

This PR doesn't add bulky configuration, just another json object with asset mapping if development team needs it and it works for everybody else without any modifications whatsoever.
And that custom webpack config part, it's super optional for those who need to tinker with the default functionality, there will always be developers like that. And I don't think it's the goal of this project to make it harder for developer to tinker and only provide One True Way™ approach. :)

[arunoda]

@alexindigo

@arunoda Do you mind to provide link to a PR or chunk of code where it solved? I'm not sure we're talking about the same thing. It's definitely not solved in the canary branch, as you can see in the red parts of my PR.

Export a simple Next.js into a static HTML version. Then deploy it into a static hosting service. See how we serve files.

Here's how we are doing it:

• Make sure we save correct pages/index.js files: https://github.com/zeit/next.js/blob/canary/server/export.js#L144
• Append index.js for all JS assets in document.js: https://github.com/zeit/next.js/blob/canary/server/document.js#L209
• Do the same as above in the page loader: https://github.com/zeit/next.js/blob/canary/lib/page-loader.js#L74

We need to move what we did in export.js to core next.js and append index.js everywhere.

It only solves one of the issues, but doesn't provide enough flexibility for developers to make it work for their deployment process. Like if they need release version instead of content hash of a file. Or they need to use commit hash for all their assets. Ability to inject custom logic for this kind of interaction between the app and the outside world is important for many teams.

I completely understand this. I didn't say this not important. My point is this solution is like a hot-fix. Not the real solution we are looking for.
Here's how the ideal solution looks like:

• We completely get rid of buildId. But instead we prefix (or postfix) each and every JS file with it's hash.
• Then you'll set assetPrefix with your S3 CDN URL (or any other CDN)
• Then you can just upload all the static statics from every releases into that S3 directory.

It'll achieve what you wanted.

[alexindigo]

@arunoda

Export a simple Next.js into a static HTML version. Then deploy it into a static hosting service. See how we serve files.

It works only for static sites, in our case (and I assume few others out there) static site is not the option, like we have listing for all the real estate properties in the US plus other related pages. Having it all exported as static pages would be somewhat suboptimal :)

In the same time we do utilize static export feature (you can see it in my examples here) to get assets structure close to how client side is requesting it, since build (.next) folder stores assets in very different structure from how it's being requested from the browser.

We need to move what we did in export.js to core next.js and append index.js everywhere.

This will solve one of the problems with inconsistent urls and it's welcome, although breaking, change. Current approach allows to have said functionality without breaking existing apps.
Also by providing custom mapping it allows developers to have more control over their assets than just appended index.js. This kind of freedom will allow future extension without breaking and lengthy pull requests to the core next.js.

Btw, I'm curious why it was chosen to have <pagename>/index.js instead of <pagename>.js.

Here's how the ideal solution looks like:

Getting rid of build id only solves one issue, it still doesn't allow developers to provide custom attributes for each asset endpoint the app is going to hit.

Only custom getAssetPath function will allow for the full control.

[arunoda]

@alexindigo

I didn't ask you to use static exports. I asked you to use give it a try and see how we serve JS files directly from S3 (or any static hosting solution) without a custom server. We can use the same technique in everywhere.

Then you don't need to have a custom-asset-map.
It's not going to be a breaking change in any case.

Btw, I'm curious why it was chosen to have /index.js instead of .js.

In the pages directory user can use either one. (But not both). We had to choose a one format to save the file. So, we(actually I) choose <pagename>/index.js.
(There was no big reason)

I'll add this support in coming days.

---

Getting rid of build id only solves one issue, it still doesn't allow developers to provide custom attributes for each asset endpoint the app is going to hit.

What custom attributes?
I assume there's a better way to deal with this.

I'd like to see a use case?

[alexindigo]

@arunoda

I didn't ask you to use static exports. I asked you to use give it a try and see how we serve JS files directly from S3 (or any static hosting solution) without a custom server. We can use the same technique in everywhere.

I played with export command quite a bit yes. The main problem for me is that it was totally independent from how app itself works.

If this part could be fixed it'd solve my immediate need for sure.

I'll add this support in coming days.

That will be awesome. Does it mean it will be landed in v4 or we'd have to wait for v5 to became stable?

What custom attributes?
I assume there's a better way to deal with this.

For example there is requirement (Enterprise is weird) :) not to have _next in the asset url or existing deployment process only works with release versions.

So instead of cdn/_next/<build id or some other smart only one true way hash>/page/<pagename>/index.js developers need to serve assets out of endpoint like cdn/v2017.45.1/<app_name>/<pagename>.js

Or there is A/B testing framework that deploys different versions of the app bundles to different paths, like cdn/<appname>/test123/a/<pagename>.js and cdn/<appname>/test123/b/<pagename>.js

It won't be as easy to incorporate all that use cases into the core, but allowing teams to augment existing behavior will open up your framework to more customers.

[arunoda]

@alexindigo

That will be awesome. Does it mean it will be landed in v4 or we'd have to wait for v5 to became stable?

We can do a minor release in 4.x branch.

[arunoda]

@alexindigo

Okay. I understand.
So, shall we refactor this to include only getAssetPath features.

[alexindigo]

@arunoda

We can do a minor release in 4.x branch.

Cool. And it should solve #2053 and #2134 issues as well.

@JeremyJonas Would that approach work for your use case?

Thanks.

[alexindigo]

@arunoda

So, shall we refactor this to include only getAssetPath features.

And that approach requires support for mapping, due to "isomorphic" nature of the code, function is static and being prebuilt for server side and client side use cases and it needs to be fed with actual data like build / commit / a/b testing version. Here comes the mapping.

So having map support as part of the original function with guide the devs on how to implement things properly and will allow "simplified" customization when things as _next/BUILD_ID is not a concern.

[JeremyJonas]

That proposal should solve my use case; adding complete filenames to server paths (same as static export) and having path control with new getAssetPath feature.

The assetMap could be implemented per-project within a custom getAssetPath function, so no need to explicitly support it.
Could exportPathMap be used in getAssetPath to reach the same goals as assetMap, with a bit of parsing in custom getAssetPath function? Should it be passed to getAssetPath?
export default function getAssetPath (hash, asset, assetPrefix = '', exportPathMap = null)

[alexindigo]

Thanks for the comment @JeremyJonas
In this PR assetMap is being passed from next.config.js all the way to the getAssetPath function in browser at runtime and this code should exist in order for custom function to work.
Probably we can use exportPathMap instead, not sure though what is the principle difference between the two.

[JeremyJonas]

@arunoda Is there an ETA for your fix on this? Is there a separate PR/issue to track? Thank you!

[arunoda]

@JeremyJonas @alexindigo
here's the fix: #3393

It's on the master and I did a new release.

Simply keep this PR with the getAssetPath() and rebase this with the master.

[alexindigo]

@arunoda After pulling in latest canary, I've got new error when starting next dev in the examples:

> next

Error: Cannot find module 'uglifyjs-webpack-plugin'
    at Function.Module._resolveFilename (module.js:536:15)
    at Function.Module._load (module.js:466:25)
    at Module.require (module.js:579:17)
    at require (internal/module.js:11:18)
    at Object.<anonymous> (/Users/alex/Projects/next.js/dist/server/build/webpack.js:61:30)
    at Module._compile (module.js:635:30)
    at Module._compile (/Users/alex/Projects/next.js/node_modules/source-map-support/source-map-support.js:492:25)
    at Object.Module._extensions..js (module.js:646:10)
    at Module.load (module.js:554:32)
    at tryModuleLoad (module.js:497:12)

[alexindigo]

Looks like related: #3398

[alexindigo]

@arunoda @timneutkens @rauchg I rebased with fixed canary and it's working now. Please take a look. Thank you.

[alexindigo]

Hang on, found missing piece, it fell through cracks due to multiple rebases :)

[alexindigo]

Fixed it.

[alexindigo]

@arunoda @rauchg Wondering if you had a chance to look into this. Thanks.

[jgautheron]

@alexindigo Thanks for contributing this feature, it'll solve a problem I have with performance when running Next in Lambda@Edge. Hopefully it'll get merged soon 👍