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

fix(env-replacer): don't modify string literals #1943

Merged
merged 5 commits into from Sep 1, 2022
Merged

fix(env-replacer): don't modify string literals #1943

merged 5 commits into from Sep 1, 2022

Conversation

tony19
Copy link
Contributor

@tony19 tony19 commented Aug 31, 2022

fix #1941

Copy link
Member

@sheremet-va sheremet-va left a comment

Choose a reason for hiding this comment

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

Can you add a test case for this?

@tony19
Copy link
Contributor Author

tony19 commented Aug 31, 2022

Sure. I don't see any plugin-specific tests in test/. Where should I test this?

@antfu
Copy link
Member

antfu commented Aug 31, 2022

You can add a new test that contains import.meta.env as an string and do assertions to ensure it's not been replaced

@sheremet-va
Copy link
Member

Sure. I don't see any plugin-specific tests in tests/. Where should I test this?

I think we have env.test.ts

@tony19
Copy link
Contributor Author

tony19 commented Aug 31, 2022

The test-ui job is failing when starting Cypress due to cypress-io/cypress#22795:

Run pnpm run ui:test

> @vitest/monorepo@0.22.1 ui:test /home/runner/work/vitest/vitest
> npm -C packages/ui run test:run


> @vitest/ui@0.22.1 test:run
> cypress run --component

It looks like this is your first time using Cypress: 10.6.0

[STARTED] Task without title.
[SUCCESS] Task without title.

Opening Cypress...
[22[7](https://github.com/vitest-dev/vitest/runs/8107375777?check_suite_focus=true#step:6:8)4:0[8](https://github.com/vitest-dev/vitest/runs/8107375777?check_suite_focus=true#step:6:9)31/061601.600053:ERROR:gpu_memory_buffer_support_x11.cc(44)] dri3 extension not supported.
(node:2285) ExperimentalWarning: Custom ESM Loaders is an experimental feature. This feature could change at any time
(Use `node --trace-warnings ...` to show where the warning was created)
(node:2285) ExperimentalWarning: The Node.js specifier resolution flag is experimental. It could change or be removed at any time.
Error [ERR_LOADER_CHAIN_INCOMPLETE]: "file:///home/runner/.cache/Cypress/10.6.0/Cypress/resources/app/node_modules/ts-node/esm/transpile-only.mjs 'resolve'" did not call the next hook in its chain and did not explicitly signal a short circuit. If this is intentional, include `shortCircuit: true` in the hook's return.
    at new NodeError (node:internal/errors:387:5)
    at ESMLoader.resolve (node:internal/modules/esm/loader:852:13)
    at async ESMLoader.getModuleJob (node:internal/modules/esm/loader:431:7)
    at async Promise.all (index 0)
    at async ESMLoader.import (node:internal/modules/esm/loader:533:24)
    at async loadESM (node:internal/process/esm_loader:[9](https://github.com/vitest-dev/vitest/runs/8107375777?check_suite_focus=true#step:6:10)1:5)
    at async handleMainPromise (node:internal/modules/run_main:65:[12](https://github.com/vitest-dev/vitest/runs/8107375777?check_suite_focus=true#step:6:13)) {
  code: 'ERR_LOADER_CHAIN_INCOMPLETE'
}
Error: The operation was canceled.

@antfu
Copy link
Member

antfu commented Aug 31, 2022

You can ignore the Cypress error for now

@tony19
Copy link
Contributor Author

tony19 commented Sep 1, 2022

The Windows test failure is unrelated to my changes, as it's also happening on the main branch.

I get a different test failure locally. Looks like a snapshot needs to be updated:

 FAIL  test/runner.test.ts > should fails > timeout.test.ts
 ❯ test/runner.test.ts:36:18
AssertionError: Snapshot `should fails > timeout.test.ts > timeout.test.ts 1` mismatched
     34|         ?.trim()
     35|         .replace(root, '<rootDir>')
     36|       expect(msg).toMatchSnapshot(file)
       |                  ^
     37|     }, 10000)
     38|   }

  - Expected   ""Error: Test timed out in 10ms.""
  + Received   ""Error: Hook timed out in 10ms.""

After hitting u to update the snapshot, all tests pass for me on macOS.

@sheremet-va sheremet-va merged commit 5182489 into vitest-dev:main Sep 1, 2022
@tony19 tony19 deleted the fix/env-replacer-not-string-literals branch September 1, 2022 10:49
antfu pushed a commit to poyoho/vitest that referenced this pull request 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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

vite:env-replacer incorrectly replaces import.meta.env inside string literals and comments
3 participants