Skip to content
This repository was archived by the owner on Jan 28, 2025. It is now read-only.

Conversation

@jlaramie
Copy link
Contributor

  • Removed next build
  • Updated README with changes
  • Added missing inputs to README
  • Clarified the use of nextConfigDir
  • Modified deploy step

- Removed `next build`
- Updated README with changes
 - Added missing `inputs` to README
 - Clarified the use of `nextConfigDir`
 - Modified `deploy` step
@jlaramie
Copy link
Contributor Author

jlaramie commented Oct 22, 2019

@danielcondemarin I added a new input nextStaticDir to deal with the use case when static or public directory is not a direct child of the nextConfigDir.

Right now it is setup so nextConfigDir and nextStaticDir are merged in determining the location.

…of appending to it. I think this is clearer to configure.
@jlaramie
Copy link
Contributor Author

jlaramie commented Oct 23, 2019

I modified this so that instead of merging the two values the nextStaticDir is used instead of nextConfigDir

@jlaramie
Copy link
Contributor Author

@danielcondemarin What is the status here. nextStaticDir is an important configuration option for anyone with a more complicated nextjs directory structure.

@danielcondemarin
Copy link
Contributor

@jlaramie Sorry it has taken me a while to look at this.

Could you expand on why you need a custom nextStaticDir? And provide an example file tree please. I just want to make sure whatever we add to the component is actually possible to do in next.

@jlaramie
Copy link
Contributor Author

jlaramie commented Nov 30, 2019

@danielcondemarin Yeah so I started doing this with my other NextJS based apps as a way to seperate some of the dev directories (.next, node_modules, etc) from the source itself.

This is a breakdown of my directory structure

./frontend
./frontend/next.config.js
./frontend/src/pages/
./node_modules
./package.json
./webpack.config.js

My next and next-build commands have frontend/src as an argument and my distDir in next.config.js is distDir: "../../.next",.

The reason why nextStaticDir needs to exist is the fact the next.js allows you to configure the location of your src directory and of your distDir directory.

It is kind of an edge case but if anyone calls next build path/to/src it will not deploy properly as the static directory will not be in the same location as the next.config.js file.

Here is the exact scripts I have configured in my package.json

  "scripts": {
    "start": "next frontend/src",
    "build": "yarn build-next && serverless",
    "logs": "serverless logs",
    "build-next": "yarn next-build && yarn export-sitemap",
    "next-build": "next build frontend/src",
    "export-sitemap": "node export-sitemap.js",
    "postinstall": "cd ./node_modules/serverless-nextjs-monorepo/packages/serverless-nextjs-component && yarn",
    "analyze": "cross-env BUNDLE_ANALYZE=both yarn next-build",
    "analyze:server": "cross-env BUNDLE_ANALYZE=server yarn next-build",
    "analyze:browser": "cross-env BUNDLE_ANALYZE=browser yarn next-build"
  },

@n8jadams
Copy link

n8jadams commented Dec 11, 2019

I just spend a bunch of time rewriting this component, only to find this beautiful PR that's already done!

I just tested this out with the following file structure:

├── client
│   ├── next.config.js
│   ├── public
│   │   ├── favicon.ico
│   │   └── styles.css
│   └── src
│       └── pages
│           └── index.js
├── package.json
└── serverless.yml

and these contents of my serverless.yml

myNextApplication:
  component: "/path/to/local/instance/of/serverless-nextjs-component"
  inputs:
    nextConfigDir: "./client"

and the following npm "script" in my package.json:

"scripts": {
    "deploy": "next build ./client && serverless"
}

and when I ran npm run deploy, it worked flawlessly. Please merge and publish this change.

@jlaramie
Copy link
Contributor Author

Thanks @n8jadams I"ve been using this since I made the PR and have not had any problems.

@danielcondemarin
Copy link
Contributor

@jlaramie Thank you for the great explanation and sorry things have moved so slow.
I now understand the current limitation.
Would you be happy with me merging #213? I want to give a bit more thought to decoupling the build from deployment which this PR implements.

@jlaramie
Copy link
Contributor Author

@danielcondemarin If we aren't going to decouple the build then I'd like a way to pass additional ENVs to the build. As I've progressed the projects that are using this library, I've now run into needing access to serverless variables. Currently I solve this by creating my own custom component that gets fed the variables and does the NextJS builds. The outputs from the custom component get passed into the serverless-nextjs-component so that it doesn't run until my build component finishes.

I suggest something like this:

  • Support inputs.nextStaticPath
  • Support inputs.nextBuildArguments
  • Support inputs.buildScripts. This defaults to running the standard build with inputs.nextBuildArguments but can be overridden with an array of scripts to run.

Here is a mock sample of the default configuration

MyNextJSProject:
  component: serverless-next.js
  inputs:
    build: true
    nextStaticDir: './'
    buildScripts:
      - cmd: 'node_modules/.bin/next'
        args: ['build'].concat(inputs.nextBuildArguments)
        cwd: './'
        env:
          envOne: true

Benefits

  1. Allows the build to stay coupled for basic setups
  2. Easily put additional dependencies into the build
  3. Supports two types of being overridden: Passing inputs.nextBuildArguments or by replacing the inputs.buildScripts

Sorry for the rant but I"m just trying to figure out what will get me out of this fork and back into master. I'm going to create a pull request for this.

@jlaramie
Copy link
Contributor Author

214 build override #259

@jlaramie
Copy link
Contributor Author

@n8jadams I'm closing this in favor of #259 now that it got merged

@jlaramie jlaramie closed this Dec 22, 2019
@jlaramie jlaramie deleted the 214-decouple-build branch July 2, 2021 20:08
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants