fix: esm suport

[xiaoxiangmoe]

Which problem is this PR solving?

Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. List any dependencies that are required for this change.

Fixes # (issue)

Short description of the changes

Type of change

Please delete options that are not relevant.

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• This change requires a documentation update

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration

• Test A

check compiled dist has no require('...') expression

Checklist:

• Followed the style guidelines of this project
• Unit tests have been added
• Documentation has been updated

[linux-foundation-easycla]

• ✅login: xiaoxiangmoe / (a5a9655)

The committers listed above are authorized under a signed CLA.

[Netail]

The requires were inline for lazy loading

[xiaoxiangmoe]

@Netail This is not too much code so it it not needed for use lazy loading. Also, use require in esm is illegal.

[pichlermarc]

@Netail This is not too much code so it it not needed for use lazy loading.

It's there to avoid loading things that may not exist on all platforms (see failing tests looks like the tests are not failing here - there may be some gaps in our tests)

Also, use require in esm is illegal.

ACK - please change the code to use dynamic imports instead to keep in line with the original intentions of the code.

[codecov]

Codecov Report

Attention: Patch coverage is 54.54545% with 5 lines in your changes missing coverage. Please review.

Project coverage is 94.59%. Comparing base (90afa28) to head (a5a9655).
Report is 2 commits behind head on main.

Files with missing lines	Patch %	Lines
...detectors/platform/node/machine-id/getMachineId.ts	54.54%	5 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #5298      +/-   ##
==========================================
- Coverage   94.62%   94.59%   -0.04%     
==========================================
  Files         323      324       +1     
  Lines        8068     8078      +10     
  Branches     1637     1637              
==========================================
+ Hits         7634     7641       +7     
- Misses        434      437       +3     

Files with missing lines	Coverage Δ
...detectors/platform/node/machine-id/getMachineId.ts	47.82% <54.54%> (+12.53%)	⬆️

... and 1 file with indirect coverage changes

[chancancode]

It's there to avoid loading things that may not exist on all platforms (see failing tests looks like the tests are not failing here - there may be some gaps in our tests)

I took a look during the holidays: in this particular case as the code is currently written, they don't import any platform-specific feature, they only try to run a command and parse the result, so won't blow up until the function is called, that's why.

Functionally it probably wouldn't have been a big deal to refactor/consolidate the modules, but agreed that it is bad form to drop this for no other reason if dynamic imports work on the supported platform.

[xiaoxiangmoe]

👌 I'll create a fix to using dynamic import