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

vite:env-replacer incorrectly replaces import.meta.env inside string literals and comments #1941

Closed
6 tasks done
tony19 opened this issue Aug 31, 2022 · 1 comment · Fixed by #1943
Closed
6 tasks done

Comments

@tony19
Copy link
Contributor

tony19 commented Aug 31, 2022

related vitejs/vite#9791

Describe the bug

The vite:env-replacer plugin is indiscriminately replacing import.meta.env everywhere in a source file, including string literals and comments.

For example, this test file:

// MyComponent.spec.js
test('ignores import.meta.env', () => {})

...gets transformed into:

// MyComponent.spec.js
test('ignores process.env', () => {})

Another example:

// importMetaEnv.test.ts
it('import.meta.env equals import' + '.meta.env', () => {
  expect('import.meta.env').toBe('import' + '.meta.env') // ❌ fails
})

...which results in:

 ❯ test/importMetaEnv.test.ts (1)
   ❯ process.env (1)
     × process.env equals import.meta.env

 ❯ eval test/importMetaEnv.test.ts:5:30
      3| describe('import.meta.env', () => {
      4|   it('import.meta.env equals import' + '.meta.env', () => {
      5|     expect('import.meta.env').toBe('import' + '.meta.env');
       |                              ^
      6|   });
      7| });

⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯ Failed Tests 1 ⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯

 FAIL  test/importMetaEnv.test.ts > process.env > process.env equals import.meta.env
AssertionError: expected 'process.env' to be 'import.meta.env' // Object.is equality
  - Expected   "import.meta.env"
  + Received   "process.env"

Test Files  1 failed (1)
     Tests  1 failed (1)
  Start at  23:19:24
  Duration  49ms

Workaround

Use string concatenation to avoid the string replacement (as was done in the importMetaEnv.test.ts above):

'import' + '.meta.env'

Solution 1: Strip literals from analysis

Use strip-literal on the string being scanned for import.meta.env:

-const envs = code.matchAll(/\bimport\.meta\.env\b/g)
+if (!/\bimport\.meta\.env\b/g.test(code))
+  return null
+
+const envs = stripLiteral(code).matchAll(/\bimport\.meta\.env\b/g)

This would mimic the way Vite does it.

Solution 2: Remove EnvReplacer; use Vite env instead

Vite already replaces import.meta.env in its vite:define plugin and vite:import-analysis plugin, so this plugin seems unnecessary.

Reproduction

StackBlitz

System Info

System:
    OS: macOS 11.6.7
    CPU: (8) x64 Intel(R) Core(TM) i7-6820HQ CPU @ 2.70GHz
    Memory: 259.93 MB / 16.00 GB
    Shell: 5.8 - /bin/zsh
  Binaries:
    Node: 16.17.0 - ~/n/bin/node
    Yarn: 1.22.19 - ~/n/bin/yarn
    npm: 8.15.0 - ~/n/bin/npm
  Browsers:
    Chrome: 104.0.5112.101
    Chrome Canary: 107.0.5271.0
    Firefox: 103.0.2
    Safari: 15.6
  npmPackages:
    vite: ^3.0.9 => 3.0.9 
    vitest: workspace:* => 0.22.1

Used Package Manager

pnpm

Validations

@sheremet-va
Copy link
Member

sheremet-va commented Aug 31, 2022

Vite replaces it to static variables, but we replace it to dynamic process.env, so users can redefine it in runtime, so solution 2 will break its usage.

Solution 1 is ok, PR welcome.

sheremet-va pushed a commit that referenced this issue Sep 1, 2022
* fix(env-replacer): don't modify string literals

fix #1941

* test(env-replacer): verify string literals are ignored

* chore: fix typo envRelacer.ts -> envReplacer.ts

* chore: fix typo envRelacer.ts -> envReplacer.ts

* chore: move strip-literal to correct package
antfu pushed a commit to poyoho/vitest that referenced this issue Sep 4, 2022
* fix(env-replacer): don't modify string literals

fix vitest-dev#1941

* test(env-replacer): verify string literals are ignored

* chore: fix typo envRelacer.ts -> envReplacer.ts

* chore: fix typo envRelacer.ts -> envReplacer.ts

* chore: move strip-literal to correct package
@github-actions github-actions bot locked and limited conversation to collaborators Jun 13, 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.

2 participants