-
Notifications
You must be signed in to change notification settings - Fork 0
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
Bump to v1.0.2, add .js extension to local imports #20
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
After installing v1.0.1 in a local copy of mbland/tomcat-servlet-testing-example, running the tests against jsdom broke with: ```text FAIL main.test.js [ main.test.js ] Error: Cannot find module '.../tomcat-servlet-testing-example/strcalc/src/main/frontend/node_modules/.pnpm/[email protected]/node_modules/test-page-opener/lib/browser' imported from .../tomcat-servlet-testing-example/strcalc/src/main/frontend/node_modules/.pnpm/[email protected]/node_modules/test-page-opener/index.js ⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯ Serialized Error: { code: 'ERR_MODULE_NOT_FOUND', url: 'file:///.../tomcat-servlet-testing-example/strcalc/src/main/frontend/node_modules/.pnpm/[email protected]/node_modules/test-page-opener/lib/browser' } ``` Running `pnpm link ../../../../../test-page-opener` and running the tests again worked, so I wouldn't've caught this before releasing v1.0.1, either. And apparently this is quite a thing, especially impacting TypeScript users: - https://www.google.com/search?q=node+import+requires+.js+extension - nodejs/node#46006 - https://nodejs.org/docs/latest-v18.x/api/esm.html#import-specifiers So be it.
Pull Request Test Coverage Report for Build 7427355354
💛 - Coveralls |
mbland
added a commit
to mbland/tomcat-servlet-testing-example
that referenced
this pull request
Jan 6, 2024
Required adding test-page-opener to the test.server.deps.inline config variable. Also updated all internal import paths to end with the '.js' extension. These changes were a consequence of the fact that the previous test/page-loader.js file was automatically bundled via Rollup, and the "external" test-page-opener isn't by default. Without bundling, test-page-opener can't resolve Handlebars modules transformed via rollup-plugin-handlebars-precompiler. --- On first adding test-page-opener to the project, running main.test.js in the browser passed. When running it in Node, all of the internal imports that didn't end with '.js' failed. This followed on the heels of mbland/test-page-opener#20, which I created when test-page-opener's own internal imports failed when used in this project. (I also filed mbland/test-page-opener#21 when its internal istanbul-lib-coverage import broke in this project, too.) After updating all the local imports on .js files, the Handlebars imports (ending in '.hbs') continued to fail with ERR_UNKNOWN_FILE_EXTENSION: ```sh % pnpm test -- run main.test.js RUN v1.1.3 /.../tomcat-servlet-testing-example/strcalc/src/main/frontend ❯ main.test.js (1) ❯ String Calculator UI on initial page load (1) × contains the "Hello, World!" placeholder —— Failed Tests 1 —— FAIL main.test.js > String Calculator UI on initial page load > contains the "Hello, World!" placeholder Error: opening index.html ❯ JsdomPageOpener.open node_modules/.pnpm/[email protected]/node_modules/test-page-opener/lib/jsdom.js:85:13 ❯ TestPageOpener.open node_modules/.pnpm/[email protected]/node_modules/test-page-opener/index.js:109:18 ❯ main.test.js:18:26 16| 17| test('contains the "Hello, World!" placeholder', async () => { 18| const { document } = await opener.open('index.html') | ^ 19| const appElem = document.querySelector('#app') 20| Caused by: Error: importing file:///.../tomcat-servlet-testing-example/strcalc/src/main/frontend/main.js ❯ node_modules/.pnpm/[email protected]/node_modules/test-page-opener/lib/jsdom.js:173:25 ❯ importModulesOnEvent node_modules/.pnpm/[email protected]/node_modules/test-page-opener/lib/jsdom.js:131:15 Caused by: TypeError: Unknown file extension ".hbs" for /.../mbland/tomcat-servlet-testing-example/strcalc/src/main/frontend/components/calculator.hbs ⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯ Serialized Error: { code: 'ERR_UNKNOWN_FILE_EXTENSION' } ⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯ ``` (I'm rather relieved to have implemented mbland/test-page-opener#14 just today, which is why there are such nice "Caused by:" messages above.) When I temporarily replaced the npm dependency with my local working copy via `pnpm link ../../../../../test-page-opener`, the test would pass again. It occurred to me that the Node runs of the test weren't including test-page-opener in the Rollup bundle containing the transformed .hbs files. Or, it would include it when it was linked to the local copy, but not include it when installed normally. I eventually figured out that the Vitest config setting server.deps.inline would cause the "external" test-page-opener to get bundled, enabling it to resolve the '.hbs' imports. - <https://vitest.dev/config/#server-deps-inline>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
After installing v1.0.1 in a local copy of
mbland/tomcat-servlet-testing-example, running the tests against jsdom broke with:
Running
pnpm link ../../../../../test-page-opener
and running the tests again worked, so I wouldn't've caught this before releasing v1.0.1, either.And apparently this is quite a thing, especially impacting TypeScript users:
So be it.