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

Failing "error in endpoint" test for Node 14.12 and below #354

Closed
benmccann opened this issue Jan 31, 2021 · 8 comments
Closed

Failing "error in endpoint" test for Node 14.12 and below #354

benmccann opened this issue Jan 31, 2021 · 8 comments

Comments

@benmccann
Copy link
Member

The error in endpoint test is failing for me when run locally. I added a message in #353 that helps show what's happening though I don't know what's causing it

   FAIL  dev  "error in endpoint [no js]"
    Could not find endpoint.svelte:11:15 in Error: Internal Server Error
    at load (/_app/routes/errors/endpoint.svelte.js:14:11)
    at async get_response (packages/kit/src/renderer/page.js:150:11)
    at async render_page (packages/kit/src/renderer/page.js:339:14)
    at async Object.render (packages/kit/src/renderer/index.js:25:26)
    at async packages/kit/dist/index2.js:11842:22  (ok)

   FAIL  dev  "error in endpoint [js]"
    Could not find endpoint.svelte:11:15 in Error: Internal Server Error
    at load (/_app/routes/errors/endpoint.svelte.js:14:11)
    at async get_response (packages/kit/src/renderer/page.js:150:11)
    at async render_page (packages/kit/src/renderer/page.js:339:14)
    at async Object.render (packages/kit/src/renderer/index.js:25:26)
    at async packages/kit/dist/index2.js:11842:22  (ok))
@Rich-Harris
Copy link
Member

Is this still failing?

@benmccann
Copy link
Member Author

I tried to test again just now, but pnpm test is failing altogether for me now:

packages/kit/src/renderer/page.js:2
import fetch, { Response } from 'node-fetch';
                ^^^^^^^^
SyntaxError: The requested module 'node-fetch' is expected to be of type CommonJS, which does not support named exports. CommonJS modules can be imported by importing the default export.
For example:
import pkg from 'node-fetch';
const { Response } = pkg;
    at ModuleJob._instantiate (internal/modules/esm/module_job.js:98:21)
    at async ModuleJob.run (internal/modules/esm/module_job.js:137:5)
    at async Loader.import (internal/modules/esm/loader.js:165:24)
    at async Module.run (node_modules/.pnpm/uvu@0.5.1/node_modules/uvu/run/index.mjs:10:3)
    at async node_modules/.pnpm/uvu@0.5.1/node_modules/uvu/bin.js:26:5
packages/kit:
 ERROR  @sveltejs/kit@1.0.0-next.37 test: `npm run test:unit && npm run test:integration`
Exit status 1
 ERROR  Test failed. See above for more details.

@Rich-Harris
Copy link
Member

Huh. What Node version? Working fine for me on 12

@benmccann
Copy link
Member Author

I'm on Node v14.8.0, npm 7.5.6, pnpm 5.18.3. I tested from a clean build by running: git clean -xdf, pnpm install, pnpm build, pnpm test

@pngwn
Copy link
Member

pngwn commented Mar 7, 2021

All fine for me. Tested on both node@12 and 14.

@benmccann
Copy link
Member Author

I got a new machine and it's passing there, so I'll close this since it's also passing for you guys

@GrygrFlzr
Copy link
Member

GrygrFlzr commented Mar 8, 2021

Based on @benmccann's comments I suspected an issue with the specific node version. I downgraded my node version from 14.15.3 to 14.8.0 and was able to replicate this without needing to re-do git clean / pnpm install / pnpm build. I then progressively updated my node version to locate the minimum version at which the tests could run without the reported error.

  • 14.8.0: FAIL
  • 14.9.0: FAIL
  • 14.10.0: FAIL
  • 14.11.0: FAIL
  • 14.12.0: FAIL
  • 14.13.0: SUCCESS
  • 14.15.3: SUCCESS
  • 14.16.0: SUCCESS

Temporarily reopening until there's a clear minimum supported Node version policy with regards to patch versions. For Node 12, we already require 12.17.0+, but I don't know if package.json supports specifying a second version requirement for 14.13.0+, or if we can exclude the unsupported Node 13.

EDIT
Looking at the Node 14.13 changelog:

Notable Changes

  • [19b95a7fa9] - (SEMVER-MINOR) deps: upgrade to libuv 1.40.0 (Colin Ihrig) #35333
  • [f551f52f83] - (SEMVER-MINOR) module: named exports for CJS via static analysis (Guy Bedford) #35249
  • [505731871e] - (SEMVER-MINOR) module: exports pattern support (Guy Bedford) #34718
  • [0d8eaa3942] - (SEMVER-MINOR) src: allow N-API addon in AddLinkedBinding() (Anna Henningsen) #35301

It's probably related to the named exports for CJS via static analysis change.

@GrygrFlzr GrygrFlzr reopened this Mar 8, 2021
@GrygrFlzr GrygrFlzr changed the title Failing "error in endpoint" test Failing "error in endpoint" test for Node 14.12 and below Mar 8, 2021
@GrygrFlzr
Copy link
Member

Turns out it does have a syntax for such a version requirement! I've opened a PR to add those requirements, let me know if we want a stricter minimum version.

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 a pull request may close this issue.

4 participants