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(generator): add ignoreEnv generate option during ensureBuild(cmd) #8955

Merged
merged 5 commits into from May 26, 2021

Conversation

wlarch
Copy link

@wlarch wlarch commented Mar 8, 2021

Types of changes

  • Bug fix (a non-breaking change which fixes an issue)
  • New feature (a non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Description

Added a build option ignoreEnv (boolean) to ignore env and process.env fields within the ensureBuild(cmd) quick diff function of the generate.js utils.

We are using Nuxt.js to generate multiple static websites with incremental builds. The same Nuxt project (code base) is used for different Websites. The nuxt generate command receives themeOptions parameters in our build server thus, changing the environment every time the application needs to achieve a partial build. When the environment changed (env or process.env) webpack needs to rebuild the entire project, this increases yarn generate time by +30 seconds (compared to 3 seconds when webpack build is not needed). In our scenario, a webpack rebuild is needed when the code base changes, or Nuxt version, or when --force-build is specified.

Example config :

build: {
    ignoreEnv: true,
},

Another reason this change is needed is because there was no other way to modify the static cacheDir prior to nuxt ready hook. As we edit nuxt.config.js using a custom module, no appropriate hook could be used to edit the 'static: { cacheDir } option.

There are no open issues regarding this PR, but it could be related to the one related to single route generation in which I participated in the discussion :
#6138

(More information to come regarding our incremental generation with Nuxt.js).

Checklist:

@wlarch wlarch changed the title Added ignoreFields build option to ignore env & process.env during ensureBuild(cmd) Added ignoreEnv build option to ignore env & process.env during ensureBuild(cmd) Mar 8, 2021
@wlarch wlarch changed the title Added ignoreEnv build option to ignore env & process.env during ensureBuild(cmd) fix(generator): add ignoreEnv build option during ensureBuild(cmd) Mar 8, 2021
@wlarch wlarch changed the title fix(generator): add ignoreEnv build option during ensureBuild(cmd) feat(generator): add ignoreEnv build option during ensureBuild(cmd) Mar 8, 2021
Copy link
Member

@clarkdo clarkdo left a comment

Choose a reason for hiding this comment

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

Nitpicking: I think this may be more readble:

    const fields = ['nuxtVersion', 'ssr', 'target']

    if (nuxt.options.build.ignoreEnv !== true) {
      fields.push('env', 'process.env')
    }

packages/cli/src/utils/generate.js Outdated Show resolved Hide resolved
Copy link
Member

@clarkdo clarkdo left a comment

Choose a reason for hiding this comment

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

Let's give it a defualt value false and update types defination as well.

@clarkdo clarkdo requested a review from pi0 March 10, 2021 20:08
@wlarch wlarch changed the title feat(generator): add ignoreEnv build option during ensureBuild(cmd) feat(generator): add ignoreEnv generate option during ensureBuild(cmd) Mar 10, 2021
@wlarch
Copy link
Author

wlarch commented Mar 17, 2021

Thanks for your review @clarkdo. Type definition and default value have been added. Any follow-up on this ?

@clarkdo
Copy link
Member

clarkdo commented Mar 17, 2021

Looks good to me, just waiting for other members review. Cc @nuxt/framework

@@ -0,0 +1,3 @@
import { buildFixture } from '../../utils/build'
Copy link
Member

Choose a reason for hiding this comment

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

I might be missing something, but is this used to test the feature?

Copy link
Author

Choose a reason for hiding this comment

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

To be fully honest I wasn't quite sure if this was the right way to test the feature using the suggested ignoreEnv option in nuxt.config.js.

@wlarch
Copy link
Author

wlarch commented May 25, 2021

@pi0 We are wondering when can this fix be merged in a minor version ? It will be critical to an imminent release.

@pi0 pi0 merged commit b116d0d into nuxt:dev May 26, 2021
@pi0
Copy link
Member

pi0 commented May 26, 2021

Hi @wlarch sorry for the delayed merge. You can have it (in few minutes) on nuxt-edge. Since its a new feature will land in 2.16 (next month releasee)

@wlarch
Copy link
Author

wlarch commented May 26, 2021

This is wonderful, thanks ! 🌞

@wlarch wlarch deleted the v2.15.2/ignore_env_build_option branch June 9, 2021 14:22
@pi0 pi0 mentioned this pull request Aug 11, 2021
@danielroe danielroe added the 2.x label Jan 18, 2023
@danielroe danielroe mentioned this pull request Jan 19, 2023
@danielroe danielroe mentioned this pull request Feb 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants