Draft: Deep import HTML and non-ESM files with exports and `?url` query #7097

MadLittleMods · 2022-02-26T10:04:17Z

Deep import HTML and non-ESM files with exports and ?url query

Related to #6725 -> #7073 which fixed up some cases like import deepPath from 'resolve-exports-path/deep.js?url'

Relevant to this discussion: #6459

Fix element-hq/hydrogen-web#686 -> element-hq/hydrogen-web#693

TODO: This is just tracking some tests atm. Still working out a fix.

Dev notes

pnpm run test-serve -- resolve

pnpm run test-serve -- resolve --silent=false --verbose --runInBand

pnpm run debug-serve -- --runInBand resolve

When using pnpm run debug-serve, in order to get the Chromium window with the playwright results to not just close before you can see the results and inspect the devtools console there, add a debugger statement to the afterAll hook in scripts/jestPerTestSetup.ts#L162

It would be really nice if those console errors were piped up to the normal Jest logging to see.

The "Debugging" section of the contributing guide doesn't work for me as I don't see the "Javascript Debug Terminal" button in VSCode. Is there supposed to be a launch.json somewhere to define what goes on there? Perhaps I need to be on a .js or .ts file first but I was on index.html for the tests I wanted to debug.

Trying to run the test suite on Windows 10, I encountered a problem with symlinks. As suggested by lovell/sharp#1671 (comment), running in a console tab as admin works 🤷

$ pnpm run test

> vite-monorepo@ test C:\Users\MLM\Documents\GitHub\vite
> run-s test-serve test-build


> vite-monorepo@ test-serve C:\Users\MLM\Documents\GitHub\vite
> jest

Error: Jest: Got error running globalSetup - C:\Users\MLM\Documents\GitHub\vite\scripts\jestGlobalSetup.cjs, reason: EPERM: operation not permitted, symlink 'C:\Users\MLM\Documents\GitHub\vite\packages\plugin-vue-jsx' -> 'C:\Users\MLM\Documents\GitHub\vite\packages\temp\worker\node_modules\@vitejs\plugin-vue-jsx'
 ELIFECYCLE  Command failed with exit code 1.
ERROR: "test-serve" exited with 1.
 ELIFECYCLE  Test failed. See above for more details.

Trying to use pnpm run debug-serve, shows me the following error in the devtools console but this can be solved by updating the path for jest in the package.json, see jestjs/jest#3750 (comment)

Fix: "debug-serve": "cross-env VITE_DEBUG_SERVE=1 node --inspect-brk ./node_modules/.bin/jest", -> "debug-serve": "cross-env VITE_DEBUG_SERVE=1 node --inspect-brk ./node_modules/jest/bin/jest.js",

Error:

...
basedir=$(dirname "$(echo "$0" | sed -e 's,\\,/,g')")
          ^^^^^^^
SyntaxError: missing ) after argument list
...

Description

Additional context

What is the purpose of this pull request?

Bug fix
New Feature
Documentation update
Other

Before submitting the PR, please make sure you do the following

Read the Contributing Guidelines.
Read the Pull Request Guidelines and follow the Commit Convention.
Check that there isn't already a PR that solves the problem the same way to avoid creating a duplicate.
Provide a description in this PR that addresses what the PR is solving, or reference the issue that it solves (e.g. fixes #123).
Ideally, include relevant tests that fail without this PR but pass with it.

…rts w/ query Related to vitejs#6725 which fixed up some cases like `import deepPath from 'resolve-exports-path/deep.js?url'`

MadLittleMods · 2022-02-26T10:07:28Z

packages/playground/resolve/index.html

@@ -107,6 +116,18 @@ <h2>resolve package that contains # in path</h2>
  import deepPath from 'resolve-exports-path/deep.json?url'
  text('.exports-deep-query', deepPath)

+  // deep import SVG w/ exports w/ query
+  import deepSvgPath from 'resolve-exports-path/deep.svg?url'
+  text('.exports-deep-svg-query', deepSvgPath)


This one works 👍 So we can rule out the problem of it only working with .js or .json files

MadLittleMods · 2022-02-26T10:23:53Z

packages/playground/resolve/index.html

+
+  // deep import non-ESM w/ exports w/ query
+  import deepNonEsmPath from 'resolve-exports-path/deep-non-esm.js?url'
+  text('.exports-deep-non-esm-query', deepNonEsmPath)


Without any exports defined, this kind of thing normally works.

But when exports is added for the same file, then you start running into errors like:

Uncaught SyntaxError: The requested module '/@fs/C:/Users/MLM/Documents/GitHub/vite/packages/playground/resolve/exports-path/deep-non-esm.js' does not provide an export named 'default'

MadLittleMods · 2022-02-26T11:04:36Z

packages/playground/resolve/index.html

+
+  // deep import HTML w/ exports w/ query
+  import deepHtmlPath from 'resolve-exports-path/deep.html?url'
+  text('.exports-deep-html-query', deepHtmlPath)


Without any exports defined, this kind of thing normally works.

But when exports is added for the same file, then you start running into errors like:

[plugin:vite:import-analysis] Failed to parse source for import analysis because the content contains invalid JS syntax. You may need to install appropriate plugins to handle the .html file format. C:/Users/MLM/Documents/GitHub/vite/packages/playground/resolve/exports-path/deep.html:4:19 2 | <html> 3 | <body> 4 | <div>foo</div> | ^ 5 | </body> 6 | </html> at formatError (C:\Users\MLM\Documents\GitHub\vite\packages\vite\src\node\server\pluginContainer.ts:315:31) at TransformContext.error (C:\Users\MLM\Documents\GitHub\vite\packages\vite\src\node\server\pluginContainer.ts:305:13) at TransformContext.transform (C:\Users\MLM\Documents\GitHub\vite\packages\vite\src\node\plugins\importAnalysis.ts:140:14) at Object.transform (C:\Users\MLM\Documents\GitHub\vite\packages\vite\src\node\server\pluginContainer.ts:583:20) at doTransform (C:\Users\MLM\Documents\GitHub\vite\packages\vite\src\node\server\transformRequest.ts:156:27 Click outside or fix the code to dismiss. You can also disable this overlay by setting server.hmr.overlay to false in vite.config.js.

MadLittleMods · 2022-02-26T11:36:25Z

packages/playground/resolve/__tests__/resolve.spec.ts

@@ -20,6 +20,20 @@ test('deep import with query with exports field', async () => {
  expect(await page.textContent('.exports-deep-query')).not.toMatch('fail')


cc @sapphi-red since you recently fixed up a super similar and relevant case in #7073, I was curious if you had any insights or hunches about what is additionally going wrong.

(the testing and comments here are done on the latest main with your fix)

Sorry I noticed that my fiix was broken.
I have made a PR which fixes (#7098).
This would work with .html and others.

@sapphi-red Thanks for taking a look and proposing a change already!

Spawned from vitejs#7097

test: Add failing test cases for deep import HTML and non-ESM w/ expo…

a49e198

…rts w/ query Related to vitejs#6725 which fixed up some cases like `import deepPath from 'resolve-exports-path/deep.js?url'`

MadLittleMods marked this pull request as draft February 26, 2022 10:04

MadLittleMods commented Feb 26, 2022

View reviewed changes

This was referenced Feb 26, 2022

Make the SDK friendly to locally link and develop on element-hq/hydrogen-web#693

Merged

Re-introduce CommonJS support to SDK element-hq/hydrogen-web#686

Closed

MadLittleMods commented Feb 26, 2022

View reviewed changes

sapphi-red mentioned this pull request Feb 26, 2022

fix: import with query with exports/browser field #7098

Merged

9 tasks

MadLittleMods added a commit to MadLittleMods/vite that referenced this pull request Mar 1, 2022

docs: debug Jest tests using Playwright (Chromium)

319f9de

Spawned from vitejs#7097

MadLittleMods added a commit to MadLittleMods/vite that referenced this pull request Mar 1, 2022

docs: debug Jest tests using Playwright (Chromium)

34df1b0

Spawned from vitejs#7097

MadLittleMods mentioned this pull request Mar 1, 2022

docs: debug Jest tests using Playwright (Chromium) #7129

Merged

9 tasks

MadLittleMods closed this Apr 6, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Draft: Deep import HTML and non-ESM files with exports and `?url` query #7097

Draft: Deep import HTML and non-ESM files with exports and `?url` query #7097

MadLittleMods commented Feb 26, 2022 •

edited

Loading

MadLittleMods Feb 26, 2022 •

edited

Loading

MadLittleMods Feb 26, 2022 •

edited

Loading

MadLittleMods Feb 26, 2022

MadLittleMods Feb 26, 2022

sapphi-red Feb 26, 2022

MadLittleMods Feb 26, 2022

		@@ -20,6 +20,20 @@ test('deep import with query with exports field', async () => {
		expect(await page.textContent('.exports-deep-query')).not.toMatch('fail')

Draft: Deep import HTML and non-ESM files with exports and ?url query #7097

Draft: Deep import HTML and non-ESM files with exports and ?url query #7097

Conversation

MadLittleMods commented Feb 26, 2022 • edited Loading

Dev notes

Description

Additional context

What is the purpose of this pull request?

Before submitting the PR, please make sure you do the following

MadLittleMods Feb 26, 2022 • edited Loading

Choose a reason for hiding this comment

MadLittleMods Feb 26, 2022 • edited Loading

Choose a reason for hiding this comment

MadLittleMods Feb 26, 2022

Choose a reason for hiding this comment

MadLittleMods Feb 26, 2022

Choose a reason for hiding this comment

sapphi-red Feb 26, 2022

Choose a reason for hiding this comment

MadLittleMods Feb 26, 2022

Choose a reason for hiding this comment

Draft: Deep import HTML and non-ESM files with exports and `?url` query #7097

Draft: Deep import HTML and non-ESM files with exports and `?url` query #7097

MadLittleMods commented Feb 26, 2022 •

edited

Loading

MadLittleMods Feb 26, 2022 •

edited

Loading

MadLittleMods Feb 26, 2022 •

edited

Loading