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

[Web/Vite] to fileReplace or not? #14256

Closed
vicb opened this issue Jan 10, 2023 · 3 comments · Fixed by #14515
Closed

[Web/Vite] to fileReplace or not? #14256

vicb opened this issue Jan 10, 2023 · 3 comments · Fixed by #14515
Assignees

Comments

@vicb
Copy link
Contributor

vicb commented Jan 10, 2023

Current Behavior

When you generate a new web app using vite: nx generate @nrwl/web:application test --bundler=vite

It will generate

CREATE apps/test/src/environments/environment.prod.ts
CREATE apps/test/src/environments/environment.ts

But the configuration does not use those files:

// project.json
    "build": {
      "executor": "@nrwl/vite:build",
      "outputs": ["{options.outputPath}"],
      "defaultConfiguration": "production",
      "options": {
        "outputPath": "dist/apps/test"
      },
      "configurations": {
        "development": {
          "mode": "development"
        },
        "production": {
          "mode": "production"
        }
      }
    },

I was expecting to see something like:

      "configurations": {
        "production": {
          "fileReplacements": [
            {
              "replace": "apps/test/src/environments/environment.ts",
              "with": "apps/test/src/environments/environment.prod.ts"
            }
          ]
        }
      }

Expected Behavior

I first thought that the fileReplacements should be added the project.json.

But digging into the code it seems that the intent is to remove the src/environments/... files and have the users use import.meta.env.MODE instead.

So this line obviously doesn't work.

Github Repo

n/a

Steps to Reproduce

See the description

Nx Report

Node : 18.10.0
   OS   : linux x64
   npm  : 8.19.2
   
   nx : 15.4.5
   @nrwl/angular : Not Found
   @nrwl/cypress : 15.4.5
   @nrwl/detox : Not Found
   @nrwl/devkit : 15.4.5
   @nrwl/esbuild : Not Found
   @nrwl/eslint-plugin-nx : 15.4.5
   @nrwl/expo : Not Found
   @nrwl/express : 15.4.5
   @nrwl/jest : 15.4.5
   @nrwl/js : 15.4.5
   @nrwl/linter : 15.4.5
   @nrwl/nest : Not Found
   @nrwl/next : Not Found
   @nrwl/node : 15.4.5
   @nrwl/nx-cloud : Not Found
   @nrwl/nx-plugin : Not Found
   @nrwl/react : Not Found
   @nrwl/react-native : Not Found
   @nrwl/rollup : 15.4.5
   @nrwl/schematics : Not Found
   @nrwl/storybook : Not Found
   @nrwl/web : 15.4.5
   @nrwl/webpack : 15.4.5
   @nrwl/workspace : 15.4.5
   @nrwl/vite : 15.4.5
   typescript : 4.8.4
   ---------------------------------------
   Local workspace plugins:
   ---------------------------------------
   Community plugins:
         @nx-tools/nx-container: 4.0.2

Failure Logs

No response

Additional Information

I am not sure how this is best fixed.

One way is to make sure that deleting the src/environments/* files work as expected.

On the other hand I see no reason why not to support file replacement with vite out of the box? Users would have a similar way to work whatever the bundler they use and they could still use import.meta.env.MODE if they prefer.

(I have been using file replacement with vite and so far it works great)

@mandarini
Copy link
Member

@vicb thanks for reporting this. I want to take a look at this but also discuss this with @jaysoo. I will most probably get to this next week.

@jaysoo
Copy link
Member

jaysoo commented Jan 12, 2023

@vicb Thanks for reporting the issue. We definitely want to make sure the environments folder is not generated.

The reason we are moving away from fileReplacement is that you can achieve the same things with import.meta.env.MODE checks or import.meta.env.VITE_* checks. We kept the option around to users can migrate from the community plugin more seemlessly.

@mandarini mandarini self-assigned this Jan 20, 2023
mandarini added a commit to mandarini/nx that referenced this issue Jan 20, 2023
mandarini added a commit to mandarini/nx that referenced this issue Jan 20, 2023
mandarini added a commit to mandarini/nx that referenced this issue Jan 20, 2023
@github-actions
Copy link

github-actions bot commented Mar 3, 2023

This issue has been closed for more than 30 days. If this issue is still occuring, please open a new issue with more recent context.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 3, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants