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

feat(vite): add generatePackageJson option to vite executor (#16042) #16045

Conversation

jensbodal
Copy link
Contributor

@jensbodal jensbodal commented Apr 2, 2023

  • moves getExtraDependencies helper from esbuild package to js package

Current Behavior

No ability to generate package.json when building with vite executor

Expected Behavior

Allow ability to generate package.json using vite executor

Related Issue(s)

Example

    "build": {
      "executor": "@nrwl/vite:build",
      "outputs": ["{options.outputPath}"],
      "defaultConfiguration": "production",
      "options": {
        "outputPath": "dist/apps/api",
        "ssr": true
      },
      "configurations": {
        "development": {
          "mode": "development"
        },
        "production": {
          "generatePackageJson": true,
          "main": "apps/api/src/main.ts",
          "tsConfig": "apps/api/tsconfig.app.json",
          "mode": "production"
        }
      }
    },

@vercel
Copy link

vercel bot commented Apr 2, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
nx-dev ✅ Ready (Inspect) Visit Preview 💬 Add feedback Apr 3, 2023 6:50am

* moves getExtraDependencies helper from esbuild package to js package
@mandarini mandarini self-assigned this Apr 3, 2023
@jaysoo
Copy link
Member

jaysoo commented Apr 3, 2023

@jensbodal Thanks for the PR. I think the idea makes sense (--generatePackageJson option), but the approach is wrong because we're pulling in other options that don't affect the build (besides package.json generation).

We might have some time to look into this after Nx 16 release, which is in a few weeks.

The workaround for now is to maintain the dependencies manually. Let us know if you have any concerns.

@jaysoo jaysoo closed this Apr 3, 2023
@jensbodal
Copy link
Contributor Author

but the approach is wrong because we're pulling in other options that don't affect the build (besides package.json generation).

I just want to call out that that's what the esbuild executor does for two of the options. I had wanted to just put these options in a nested object that were specific to generatePackageJson but I didn't because I didn't want to diverge from existing implementations, and they might have some other purpose for the general build later and it would be messy to later require moving main and tsconfig to the root build options.

esbuild executor

The above are used for both generatePackageJson and esbuild.build (esbuild.build options)


If preferred I could just remove the allowed overrides that I provided and just set them statically instead of being defaults

      buildableProjectDepsInPackageJsonType = 'dependencies',
      excludeLibsInPackageJson = true,
      format = 'esm',
      generateLockfile = true,

This would just leave setting main, tsconfig, and generatePackageJson. Possibly there's a way to pull main and tsconfig from the vite configuration.

I'd like to work with you to make whatever changes in order to get this in since it's something I'd very much like to use.

@jensbodal
Copy link
Contributor Author

I can remove the tsconfig option and just generate that from ${buildConfig.root}/tsconfig.json

main is a little trickier since it's really only used to make the generated package.json "valid". I could either

  1. default this to ./src/index.js and someone could override the setting by having an existing package.json in their directory with the main field populated
  2. make generatePackageJson a string option and provide it there
  3. make generatePackageJson an object and require setting the value (and other overrides there)
  4. require having a package.json with the value set to use generatePackageJson

@jensbodal
Copy link
Contributor Author

I think I have a better idea now than continuously adding this feature to different executors: #16042 (comment)

@github-actions
Copy link

This pull request has already been merged/closed. If you experience issues related to these changes, please open a new issue referencing this pull request.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 10, 2023
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