working-directory input assumes a .next folder

[mwdiaz]

This looks like a great action that I'd like to use, but it isn't compatible with how we configure Next.js's distDir. More specifically, we have a variety of tools in our repository that use a common .build directory for outputting any build-related artifacts. So, for example, Next.js uses .build/next. While we could certainly update the variety of places (ignore files, cleanup scripts, etc.) we assume .build/next, it would be nice if there was a way for this action to accept a working directory that maps more directly to how distDir behaves instead of assuming a .next folder must exist.

I do recognize that changing the behavior of the working-directory input in this manner would be a breaking change, but I'm wondering if this sort of functionality is something you'd be amenable to adopting (either as a future version or via some other combinations of inputs that preserves the current functionality)? Thanks!

[Wise-StenRaudmets]

Yeah that's fair. I'm not completely opposed to the breaking change but I'd want to consider other options before that. Do you think it's possible for this action to read your next.config.js in working-directory and determine the distDir from that?
E.g.

import next from 'next';

const nextApp = next({});
const distDir = nextApp.options.conf?.distDir;

[mwdiaz]

Do you think it's possible for this action to read your next.config.js in working-directory...

Yeah, I believe that option could work — however my concern would be that the only way I'm aware of to get the actual loaded/parsed config is to use a private method in the NextServer class (I know this because we have code that relies on this very thing):

import next from 'next';

const nextApp = next({});
const distDir = (await nextApp.getServer?.())?.distDir;

It feels like it could be somewhat fragile and prone to break unexpectedly with Next.js updates so it may not be ideal to rely on.

[Wise-StenRaudmets]

I tested it out and yeah that seems to be the case, the example I gave doesn't actually work as I expected.
(await nextApp.getServer?.())?.distDir does work and only because Typescript doesn't really stop you from accessing private methods like you said.

But I also noticed that the next storybook plugin has to deal with the same problem and they import loadConfig from next.

So if I write something like this, it seems to point to the actual distDir:

import { PHASE_PRODUCTION_BUILD } from "next/constants.js";
import loadConfig from "next/dist/server/config.js";

const nextConfig = await loadConfig.default(
  PHASE_PRODUCTION_BUILD,
  process.cwd(),
);

console.log(nextConfig.distDir);

I'm a little bit more confident about this approach since these methods aren't marked private, but I do wonder if bundling Next.js into the action increases the size of the action dramatically.

Another alternative is that we expose a dist-dir input for the action that is relative to the working-directory so we can avoid a breaking change. Then it's up to the user to make sure it matches their declared distDir.

Let me know what you think.

[mwdiaz]

Ah, interesting! I wasn't aware of that approach to using loadConfig directly. I would agree that it seems less likely to break, and I don't know if ncc tree-shakes its builds since as you point out bundling Next.js could increase the action's size quite a bit. It also makes the action more tightly coupled to a specific version of Next.js.

Exposing an additional dist-dir sounds like a pretty decent solution which, while somewhat duplicative, does avoid a breaking change. I'm assuming working-directory would remain defaulted to the cwd and dist-dir would default to .next? And in my case I would just pass in .build/next for dist-dir. Does that sound right?

[Wise-StenRaudmets]

Yep, that makes sense to me. Perhaps dist-dir is the way to go.

[mwdiaz]

Is this a change you'd be will to make? Or would you prefer I take a stab at it?

[Wise-StenRaudmets]

I don't think I'll have time to work on this any time soon but I'd be willing to review any PRs.