[ESM] always load the ESM loader #279
Conversation
kherock
force-pushed
the
always-load-esm
branch
from
8246905
to
efd82dc
Compare
kherock
force-pushed
the
always-load-esm
branch
from
efd82dc
to
f2a815b
Compare
| const script = process.argv[1]; | ||
| const { extensions, fork, vm } = getConfig(script); | ||
|
|
||
| if (process.env.NODE_DEV_PRELOAD) { | ||
| require(process.env.NODE_DEV_PRELOAD); | ||
| } | ||
| require('./suppress-experimental'); |
There was a problem hiding this comment.
I don't think we should be hiding warnings by default.
There was a problem hiding this comment.
Although I can see now that it gets quite annoying.
There was a problem hiding this comment.
I was also trying to hold off on implementing this but I figure that this way is probably for the best since it was affecting the test output a lot!
| const require = createRequire(import.meta.url); | ||
|
|
||
| export async function load(url, context, defaultLoad) { | ||
| const getPackageType = require('get-package-type'); |
There was a problem hiding this comment.
It looks like this is only used in the unknown file extension case. Let's get it out of the critical path.
| try { | ||
| return await defaultLoad(url, context, defaultLoad); | ||
| } catch (err) { | ||
| if (err.code === 'ERR_UNKNOWN_FILE_EXTENSION') { | ||
| return { format: await getPackageType(filePath), source: null }; | ||
| } | ||
| throw err; | ||
| } |
There was a problem hiding this comment.
| try { | |
| return await defaultLoad(url, context, defaultLoad); | |
| } catch (err) { | |
| if (err.code === 'ERR_UNKNOWN_FILE_EXTENSION') { | |
| return { format: await getPackageType(filePath), source: null }; | |
| } | |
| throw err; | |
| } | |
| return defaultLoad(url, context, defaultLoad).catch(error => { | |
| if (error.code !== 'ERR_UNKNOWN_FILE_EXTENSION') throw error; | |
| return require('get-package-type')(filePath).then(format => ({ format, source: null }); | |
| }); |
|
I'll fix it up once I merge, thanks for the contribution! |
|
|
||
| export async function getFormat(url, context, defaultGetFormat) { | ||
| const getPackageType = require('get-package-type'); | ||
| const filePath = fileURLToPath(url); |
There was a problem hiding this comment.
@bjornstar @kherock this crashes on any import of built in modules now, of which the URL is in the format node:<identifier>, e.g. node:path:
TypeError [ERR_INVALID_URL_SCHEME]: The URL must be of scheme file
There was a problem hiding this comment.
I created an issue for this with a little test-case: #281
I noticed that this library only monitors ESM when the program entrypoint is a module. Because of this, CJS programs making use of dynamic
import()statements to load ESM code won't reload when the imported ESM changes.To work around this, I updated the spawn logic to always enable
--experimental-loaderregardless of the entrypoint script's format. I took special care to avoid running into nodejs/node#33226 when TypeScript files are loaded.Additionally, the approach for installing CJS hooks and intercepting
vmandchild_processcalls by replacing the main program and subsequently masking it seemed unnecessarily complicated and was breaking tests with the ESM loader enabled. I replaced this logic with a simple--requireoption appended to the end of theexecArgv.