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

ESM compatibility #4

Open
talentlessguy opened this issue Jul 16, 2020 · 4 comments
Open

ESM compatibility #4

talentlessguy opened this issue Jul 16, 2020 · 4 comments
Labels
enhancement New feature or request upstream

Comments

@talentlessguy
Copy link
Contributor

Hello, while making a simple dev server I noticed that Sosse still uses require, even in the ESM generated code

image

Node disallows using require in the ESM module. So it should be imported in the top so it properly builds to import chokidar from 'chokidar'

The full stacktrace is this:

> node main.js
(node:240789) UnhandledPromiseRejectionWarning: ReferenceError: require is not defined
    at _temp2 (file:///home/v1rtl/Coding/tinyhttp/node_modules/.pnpm/sosse@0.6.0/node_modules/sosse/dist/main.mjs:862:26)
    at _temp4 (file:///home/v1rtl/Coding/tinyhttp/node_modules/.pnpm/sosse@0.6.0/node_modules/sosse/dist/main.mjs:885:57)
    at hsr (file:///home/v1rtl/Coding/tinyhttp/node_modules/.pnpm/sosse@0.6.0/node_modules/sosse/dist/main.mjs:907:74)
    at file:///home/v1rtl/Coding/tinyhttp/examples/dev-server/main.js:4:1
    at ModuleJob.run (internal/modules/esm/module_job.js:140:23)
    at async Loader.import (internal/modules/esm/loader.js:162:24)

Sosse version: 0.6.0
Node version: 14.5.0

@katywings
Copy link
Contributor

@talentlessguy Tbh. those requires cannot be moved to the top of sosse. They have to be called dynamically after file changes - thats the essential part for the hot server reload ;).

Maybe there is a chance to switch to dynamic imports, but this is not something I can quickly change, I have to test this in the very detail of each require ^^.

@katywings katywings added the enhancement New feature or request label Jul 16, 2020
@talentlessguy
Copy link
Contributor Author

@katywings oh ok, no worries. I could maybe try to find the way to load CJS version of Sosse 🤔

@katywings
Copy link
Contributor

katywings commented Jul 17, 2020

@talentlessguy I investigated this and in the end got stuck in an upstream blocker 🤦

Hot module replace currently is only possible with require, the ("type": "module") import cache cannot be invalidated.

This is the corresponding upstream issue: nodejs/node#49442


I gonna watch that upstream issue but ESM support has to unfortunately wait for now 🙈

@katywings
Copy link
Contributor

Also something that should be watched going forward nodejs/node#49452.

@katywings katywings changed the title require is not defined in the ESM version of Sosse ESM compatibility Jul 18, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request upstream
Projects
None yet
Development

No branches or pull requests

2 participants