nodejs / nodejs/cjs-module-lexer

Provide information regarding multiple re-exports

Open
#99 1 comment 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
JavaScript
Stars
262
Forks
31
Avg merge
9d 2h
Merged PRs (30d)
5

Description

The problem I'm facing is that parse only reports the last re-export of cjs modules completely loosing the information about pre-existing ones.

For example by parsing the following code:

if (true) {
  module.exports = require("./a.cjs");
} else {
  module.exports = require("./b.cjs");
}

only the last re-export assignment is recognized and reported to the caller (reexports = ['./b.cjs' ]).

It would be very useful if all the re-exports in the code could be recognized and reported to the caller

For example as: reexports = ['./a.cjs', './b.cjs']

The above anyways could be considered ambiguous since the same result could be obtained by parsing

module.exports = {...require('./a.cjs'), ...require('./b,cjs')};

so maybe reexports could/should actually be an array or array of strings?
Meaning that parsing

if (true) {
  module.exports = {...require('./a.cjs'), ...require('./b,cjs')};
} else {
  module.exports = {...require('./c.cjs'), ...require('./d,cjs')};
}

could yield reexports = [['./a.cjs', './b.cjs' ], ['./c.cjs', './d.cjs' ]]

Minimal Reproduction

For convenience I've created a reproduction here: https://github.com/dario-piotrowicz/cjs-module-lexer-reexports-repro

Additional information
Bug?

As I mentioned, I am not sure if this could be considered a bug since the reexport resetting seems to be intentional.

Solving this might generate different problems

If this issue were to be addressed, I would imagine that the following code:

module.exports = require('./a.cjs');
module.exports = require('./b.cjs');

would also produce two re-exports... although only one is actually valid... so this is definitely something to keep in mind 😕

The lexer is content-unaware so I don't think much could be done to avoid the above arguably incorrect result, even if nesting was kept into account (which might be a stretch in the lexer) that would still not always generate perfect results, for example when parsing code like this:

if(true){
   module.exports = require('./a.cjs');
}

module.exports = require('./b.cjs');

(maybe instead of expanding reexports there could be a new field in the parse result? as extra-reexports or something like that that could host all the reexports excluding the last one? so keeping the same exact functionality the lexer has now whilst also providing callers with the extra bit of information?)

More context

To give a bit more context, I am simply trying to parse cjs code and gather informations about all the exports and re-exports that the code contains, unfortunately as mentioned in the issue the lexer currently doesn't return all possible re-exports but only the last one encountered, this is problematic in my use case as I would want to know all possible re-exports as well (and that information gets, as far as I can tell, completely lost).

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with the re-export resetting logic in lexer.js and inspect how the parse result records reexports. Run the minimal reproduction linked in the issue, then compare conditional, spread, and sequential assignments. Done requires an agreed representation and behavior for all possible re-exports, backed by coverage for those cases.

Written by the indexing model from the issue text.

Assessment

Tech stack
javascript
Domain
compilers
Issue type
Feature
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Needs clarification
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.