refactor: use fs.promises in resolve id, Part 4

[dnalborczyk]

This PR contains:

• bugfix
• feature
• refactor
• documentation
• other

Are tests included?

• yes (bugfixes and features will not be merged without tests)
• no

Breaking Changes?

• yes (breaking changes will not be merged unless absolutely necessary)
• no

List any relevant issue numbers:

Description

[dnalborczyk]

this causes tests to build in a non-deterministic order, similarly what is reported here: #4337

I have to digg thru the code to get more information. we probably need some deterministic processing queue (FIFO?) in order of the imported files during parsing.

there is also a specific test case which produces different results each time you run rollup. all results are runnable and perfectly fine, but contain different generated variable names. I think it's important to have deterministic builds (when input hasn't changed), it would also make testing way harder. let me try to find it ....

other than that, most failing tests have a different order of module ids.

[dnalborczyk]

for example his one:

npx mocha -g "deduplicates with named chunks defined by the user"

  rollup
    chunking form
      emit-file
        named-user-chunks: deduplicates with named chunks defined by the user
          ✔ generates es (38ms)
          ✔ generates cjs
          ✔ generates amd
          ✔ generates system


  4 passing (101ms)

run again

➜ npx mocha -g "deduplicates with named chunks defined by the user"


  rollup
    chunking form
      emit-file
        named-user-chunks: deduplicates with named chunks defined by the user
          ✔ generates es (38ms)
          1) generates cjs
          2) generates amd
          ✔ generates system


  2 passing (104ms)
  2 failing

  1) rollup
       chunking form
         emit-file
           named-user-chunks: deduplicates with named chunks defined by the user
             generates cjs:

      AssertionError [ERR_ASSERTION]: Expected values to be strictly equal:
+ actual - expected

  "explicit-name1.js: 'use strict';\n" +
    '\n' +
    "Object.defineProperty(exports, '__esModule', { value: true });\n" +
    '\n' +
+   "var ignored = require('./mainChunk.js');\n" +
-   "var mainChunk = require('./mainChunk.js');\n" +
    '\n' +
    '\n' +
    '\n' +
+   'exports.value = ignored.value;\n'
-   'exports.value = mainChunk.value;\n'
      + expected - actual

       explicit-name1.js: 'use strict';
       
       Object.defineProperty(exports, '__esModule', { value: true });
       
      -var ignored = require('./mainChunk.js');
      +var mainChunk = require('./mainChunk.js');
       
       
       
      -exports.value = ignored.value;
      +exports.value = mainChunk.value;
      
      at /Users/daniel/dev/github-forks/rollup/test/utils.js:219:10
      at Array.forEach (<anonymous>)
      at assertFilesAreEqual (test/utils.js:212:52)
      at assertDirectoriesAreEqual (test/utils.js:208:2)
      at generateAndTestBundle (test/chunking-form/index.js:74:2)
      at Context.<anonymous> (test/chunking-form/index.js:33:6)

  2) rollup
       chunking form
         emit-file
           named-user-chunks: deduplicates with named chunks defined by the user
             generates amd:

      AssertionError [ERR_ASSERTION]: Expected values to be strictly equal:
+ actual - expected

+ "explicit-name1.js: define(['exports', './mainChunk'], (function (exports, ignored) { 'use strict';\n" +
- "explicit-name1.js: define(['exports', './mainChunk'], (function (exports, mainChunk) { 'use strict';\n" +
    '\n' +
    '\n' +
    '\n' +
+   '\texports.value = ignored.value;\n' +
-   '\texports.value = mainChunk.value;\n' +
    '\n' +
    "\tObject.defineProperty(exports, '__esModule', { value: true });\n" +
    '\n' +
    '}));\n'
      + expected - actual

      -explicit-name1.js: define(['exports', './mainChunk'], (function (exports, ignored) { 'use strict';
      +explicit-name1.js: define(['exports', './mainChunk'], (function (exports, mainChunk) { 'use strict';
       
       
       
      - exports.value = ignored.value;
      + exports.value = mainChunk.value;
       
        Object.defineProperty(exports, '__esModule', { value: true });
       
       }));
      
      at /Users/daniel/dev/github-forks/rollup/test/utils.js:219:10
      at Array.forEach (<anonymous>)
      at assertFilesAreEqual (test/utils.js:212:52)
      at assertDirectoriesAreEqual (test/utils.js:208:2)
      at generateAndTestBundle (test/chunking-form/index.js:74:2)
      at Context.<anonymous> (test/chunking-form/index.js:33:6)

[codecov]

Codecov Report

Merging #4386 (79d06e7) into master (33a4278) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master    #4386   +/-   ##
=======================================
  Coverage   98.72%   98.72%           
=======================================
  Files         204      204           
  Lines        7315     7320    +5     
  Branches     2082     2080    -2     
=======================================
+ Hits         7222     7227    +5     
  Misses         34       34           
  Partials       59       59           

Impacted Files	Coverage Δ
cli/run/batchWarnings.ts	99.20% <100.00%> (+<0.01%)	⬆️
src/Chunk.ts	100.00% <100.00%> (ø)
src/Module.ts	100.00% <100.00%> (ø)
src/ModuleLoader.ts	100.00% <100.00%> (ø)
src/utils/resolveId.ts	93.33% <100.00%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 33a4278...79d06e7. Read the comment docs.

[lukastaegert]

I pushed some fixes. For the most part, those are test fixes, i.e. sorting watchFiles or moduleIds before asserting, because I do not think those need to be necessarily deterministic.
The main code chain was an indeterminism in the generation of chunk file and variable names for entries, as those were only determined after id resolution, while now they are prioritize by their emission, which should be rather stable.

[dnalborczyk]

I pushed some fixes. For the most part, those are test fixes, i.e. sorting watchFiles or moduleIds before asserting, because I do not think those need to be necessarily deterministic.

yeah, I agree, I was thinking the same.

The main code chain was an indeterminism in the generation of chunk file and variable names for entries, as those were only determined after id resolution, while now they are prioritize by their emission, which should be rather stable.

that's awesome! thank you for looking into this!