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

in vitest browser runner plugin with transformIndexHtml to inject importmap script manually does not work. #6394

Closed
6 tasks done
sashafirsov opened this issue Aug 25, 2024 · 5 comments · Fixed by #6725
Closed
6 tasks done
Labels
feat: browser Issues and PRs related to the browser runner p2-nice-to-have Not breaking anything but nice to have (priority)

Comments

@sashafirsov
Copy link

Describe the bug

Need the importmap available for non-js resources like SVG and HTML to be resolved natively by import.meta.resolve( path ).

For some reasin in vitest browser runner the usual plugin with transformIndexHtml to inject importmap script manually does not work.

Advised in vitejs/vite#2483 method of injecting script via browser.testerScripts is not applicable as a) import maps requires script type="importmap", b) it is injected after stateScript which causes "import maps injected after module use" error. Following request, openingthe bug on vitest.

Root cause:

  • Declarative Custom Element is HTML-centric and needs support for importmap for use by import.meta.resolve( path ) API. Vitest in browser does not support import maps.
  • vitest browser run uses static HTML instead of loading by vite build and altering it afterwards
  • using vitest to run the StoryBook interactive tests for TDD on visual components (React and Web Components). The StoryBook is not supported by vitest browser run, had to do a bit of tricks.

Solving on any level from $subj to root causes, better all, would be beneficial for frameworks and reusable libs authors.

Reproduction

<script type="importmap">
{
    "imports": {
        "embed-lib": "./demo/lib-dir/embed-lib.html"
    }
}
</script>
const url = import.meta.resolve( 'embed-lib' );
expect( url ).to.include( 'embed-lib.html' );

System Info

System:
    OS: Windows 11 10.0.22621
    CPU: (16) x64 12th Gen Intel(R) Core(TM) i7-1260P
    Memory: 1.31 GB / 15.69 GB
  Binaries:
    Node: 20.8.1 - C:\Program Files\nodejs\node.EXE
    Yarn: 1.22.22 - C:\Program Files\nodejs\yarn.CMD
    npm: 10.1.0 - C:\Program Files\nodejs\npm.CMD
    pnpm: 9.0.6 - C:\Program Files\nodejs\pnpm.CMD
  Browsers:
    Edge: Chromium (127.0.2651.74)
    Internet Explorer: 11.0.22621.3527
  npmPackages:
    @vitest/browser: ^1.6.0 => 1.6.0
    @vitest/coverage-istanbul: ^1.6.0 => 1.6.0
    vite: ^5.3.1 => 5.3.1
    vitest: ^1.6.0 => 1.6.0

Used Package Manager

yarn

Validations

@sheremet-va
Copy link
Member

For some reasin in vitest browser runner the usual plugin with transformIndexHtml to inject importmap script manually does not work.

transformIndexHtml is meant for the app HTML, Vitest doesn't use your index.html file in the root, so the processing is not applied. I don't think we should reuse it.

We do need to provide a way to use your own HTML, but I don't know how. Open to ideas.

Import maps are tricky because they are not supported by Vite, so we can't add our own support otherwise it will contradict the dev environment.

Also, I noticed you are using 1.6.0. The browser mode was rewritten in Vitest 2.0, we do not support lower versions anymore.

@sheremet-va sheremet-va added feat: browser Issues and PRs related to the browser runner p2-to-be-discussed Enhancement under consideration (priority) and removed pending triage labels Aug 26, 2024
@sashafirsov
Copy link
Author

There is a "right" and "fast" solution. The "right" is to allow to accept the web page from running web server and inject the code. This way it can be any framework or StoryBook built natively.

The "fast" solution is to alter

The hacking attempt into pathe/resolve to substitute client/tester/tester.html with app-specific one does not work as dist/ compiled code inlines the calls :(

PS. Upgrade vitest to current does not change the approach. The HTML loading code stays the same in "@vitest/browser": "^2.0.5",

@sashafirsov
Copy link
Author

Proposed PR for review:
#6424

@sashafirsov
Copy link
Author

@sheremet-va , in PR ^^ there is one of possible solutions. It can be not optimal and perhaps does not follow the current codebase conventions though. Who/how I can ask for review of principle?
If the solution is OK in general, I would update PR with CLI and docs support.

@sashafirsov
Copy link
Author

While the PR under review, there is a work around. In project root run:

node bin/vitest-browser-importmaps.mjs
// injects importmaps for module-url.test.stories.ts
import fs from 'node:fs'

const pathName = 'node_modules/@vitest/browser/dist/client/tester/tester.html';
const jsStr = fs.readFileSync(pathName).toString();
if( !jsStr.includes('importmap') ) // skip if already patched
{
    const injectedStr = jsStr.replace('</style>', `</style>
<script type="importmap">
    {
        "imports": {
            "lib-root/": "./demo/lib-dir/",
            "embed-lib": "./demo/lib-dir/embed-lib.html"
        }
    }
</script>
`);
    fs.writeFileSync(pathName, injectedStr);
}

@sheremet-va sheremet-va added p2-nice-to-have Not breaking anything but nice to have (priority) and removed p2-to-be-discussed Enhancement under consideration (priority) labels Sep 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feat: browser Issues and PRs related to the browser runner p2-nice-to-have Not breaking anything but nice to have (priority)
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

2 participants