Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: broken package.json commands, broken named imports due to node --experimental-modules #153

Merged
merged 2 commits into from
Oct 10, 2022

Conversation

varunsrin
Copy link
Member

@varunsrin varunsrin commented Oct 10, 2022

Motivation

After the previous #149, yarn start was broken with the following error:

SyntaxError: The requested module './interfaces' does not provide an export named 'RPCHandler'
    at ModuleJob._instantiate (node:internal/modules/esm/module_job:123:21)
    at async ModuleJob.run (node:internal/modules/esm/module_job:189:5)
    at async Promise.all (index 0)
    at async ESMLoader.import (node:internal/modules/esm/loader:541:24)
    at async loadESM (node:internal/process/esm_loader:91:5)
    at async handleMainPromise (node:internal/modules/run_main:65:12)

Also yarn dev and yarn bench were broken since they didn't point to the newly refactored file names.

Change Summary

The core issue is that node --experimental-modules has bad support for named exports, and seems to always fail on exporting a named interface. The hacky fix was to use export * for the file exporting the interface to unblock this. Further investigation is needed to see if there is a more robust fix.

Merge Checklist

  • The title of this PR adheres to the conventional commits standard
  • The PR has been tagged with the appropriate change type label(s) (i.e. documentation, feature, bugfix, or chore)

Additional Context

@varunsrin varunsrin added the t-bug A fix for a bug with the current system label Oct 10, 2022
@codecov
Copy link

codecov bot commented Oct 10, 2022

Codecov Report

Base: 91.06% // Head: 91.06% // No change to project coverage 👍

Coverage data is based on head (cda8ab3) compared to base (bdd742e).
Patch has no changes to coverable lines.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #153   +/-   ##
=======================================
  Coverage   91.06%   91.06%           
=======================================
  Files          41       41           
  Lines        2172     2172           
  Branches      372      372           
=======================================
  Hits         1978     1978           
  Misses        183      183           
  Partials       11       11           

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@varunsrin varunsrin changed the title fix(rpc): use export * instead of named import + export fix: broken package.json commands, broken named imports due to node --experimental-modules Oct 10, 2022
@varunsrin varunsrin merged commit d73534f into main Oct 10, 2022
@varunsrin varunsrin deleted the varunsrin/fix-import-module-err-oct-10 branch October 10, 2022 23:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
t-bug A fix for a bug with the current system
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant