module: add a condition to if in case of a bug.

[iam-frankqiu]

If the extension does not belong to the legacyExtensionFormatMap, It should throw an Error. I add it in case of a bug.

[GeoffreyBooth]

If the extension does not belong to the legacyExtensionFormatMap, It should throw an Error. I add it in case of a bug.

When could this happen? Can you please add a test for this behavior?

[iam-frankqiu]

If the extension does not belong to the legacyExtensionFormatMap, It should throw an Error. I add it in case of a bug.

When could this happen? Can you please add a test for this behavior?

I have tried writing a test file like the below:

require('../common');
const assert = require('assert');
const { defaultGetFormat} = require('internal/modules/esm/get_format');

{
  const url = new URL('file://example.com/foo/bar.js')
  assert.strictEqual(defaultGetFormat(url), 'commonjs')
}

{
  const url = new URL('file://example.com/foo/bar.whatever')
  assert.throws(() => defaultGetFormat(url), {name: 'TypeError', message: /Unknown file extension whatever/})
}

whthin test/es-module. But it showed an error Error: Cannot find module 'internal/modules/esm/get_format'
I don't know how to debug because other files can do it.

[GeoffreyBooth]

Within user code, though, what's the bug you're trying to fix?

[iam-frankqiu]

Within user code, though, what's the bug you're trying to fix?

Just like above. How can I import defaultGetFormat function in the test folder?

[GeoffreyBooth]

How can I import defaultGetFormat function in the test folder?

Your code is correct. const { defaultGetFormat } = require('internal/modules/esm/get_format'); should work.

Just like above.

I don’t understand. Just like what? Can you please clearly define the bug including steps to reproduce?

[iam-frankqiu]

I don’t understand. Just like what? Can you please clearly define the bug including steps to reproduce?

Thank you. I have committed my test code. The step is ./configure && make -j4 then ./node ./test/es-module/test-esm-get-format.js

[GeoffreyBooth]

I don't know what bug this PR is aiming to fix, or what new functionality it intends to add.

[iam-frankqiu]

I don't know what bug this PR is aiming to fix, or what new functionality it intends to add.

I have said it first. #40549 (comment)

[GeoffreyBooth]

If the extension does not belong to the legacyExtensionFormatMap, It should throw an Error.

This is not necessarily a bug. Can you please provide some user code that doesn’t behave as you’d expect in current latest Node? Including steps to reproduce, and what you expected the outcome to be.

[iam-frankqiu]

If the extension does not belong to the legacyExtensionFormatMap, It should throw an Error.

This is not necessarily a bug. Can you please provide some user code that doesn’t behave as you’d expect in current latest Node? Including steps to reproduce, and what you expected the outcome to be.

I have added it to the branch. but occurred a bug so I can't go on smoothly.

[GeoffreyBooth]

I don't understand.

If there aren't steps to reproduce a bug in a particular version of Node, I don't think we can land any fix. The problem needs to be clearly defined before we can adopt a solution.

I'm inclined to close this and not waste any more time on it, as I feel I've asked basic questions repeatedly and not received straight answers.

[GeoffreyBooth]

Please don’t dismiss a blocking review. I don’t think this should merge until it has clearly defined bug reproduction steps, that can be followed against a current release of Node in user code. Pointing to a test within this PR is insufficient.

[aduh95]

You need to add a flag for the test to work.

Suggested change

	'use strict';
	// Flags: --expose-internals
	'use strict';