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 re-export of io module #1503

Closed
wants to merge 2 commits into from
Closed

Conversation

chriskrycho
Copy link
Contributor

@chriskrycho chriskrycho commented Oct 14, 2021

The previous (CJS module) export semantics were of a namespace-style export, while the semantics introduced in c76d367 are of a single value exported as the default value instead. This broke downstream consumers who were using the namespace-style export, as documented at socketio/socket.io#4121. While the single default export should work fine (and might be preferable!), it's a breaking change, and appears to be an accidental breaking change, so this reverts to the previous semantics.

Accordingly, (re)introduce a manual 'namespace' by assigning the other exported value objects to the lookup function, thereby allowing it to be called directly, e.g. io(...), or used to access the other values, e.g. io.connect(...). Maintain the other existing exports so that the nice new ES module interface is still useful in its own right.

The kind of change this PR does introduce

  • a bug fix
  • a new feature
  • an update to the documentation
  • a code change that improves performance
  • other

Current behaviour

Accidentally switched away from namespace-style export.

New behaviour

Revert to previous namespace-style export.

Other information (e.g. related issues)

socketio/socket.io#4121 has the details and the history of my root-causing this.

The previous (CJS module) export semantics were of a *namespace*-style
export, while the semantics introduced in c76d367 are of a single value
exported as the default value instead. This broke downstream consumers
who were using the namespace-style export, as documented at
[socketio/socket.io#4121][4121]. While the single default export should
work fine (and might be preferable!), it's a breaking change, and
appears to be an *accidental* breaking change, so this reverts to the
previous semantics.
@darrachequesne
Copy link
Member

Please see my answer here: socketio/socket.io#4121 (comment)

Introduce a manual 'namespace' by assigning the other exported value
objects to the `lookup` function, thereby allowing it to be called
directly, e.g. `io(...)`, or used to access the other values, e.g.
`io.connect(...)`. Maintain the other existing exports so that the nice
new ES module interface is still useful in its own right.
@darrachequesne
Copy link
Member

Merged as 8737d0a and included in [email protected]. Thanks a lot for your work on this 👍

@darrachequesne darrachequesne added this to the 4.3.1 milestone Oct 15, 2021
@chriskrycho
Copy link
Contributor Author

Thanks for the quick turnaround!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants