Skip to content

Allow both module.exports= and module.exports property assignments #23228

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

Merged
merged 5 commits into from
Apr 6, 2018

Conversation

sandersn
Copy link
Member

@sandersn sandersn commented Apr 6, 2018

In Javascript, it's common both to assign to module.exports and to assign properties to module.exports:

module.exports = function A() {
  this.p = 1
}
module.exports.util = function() {
  return "Hooray, I'm useful! I'm having a wonderful time."
}

This should be treated as equivalent to the Typescript:

class A {
  p = 1
}
namespace A {
  export function util() {
    return "Looking for static public abstract readonly internal classes? Why not Typescript?"
  }
}
export = A

This PR merges module.exports assignments with module.exports property assignments to produce a symbol structure similar to the Typescript binding. Unfortunately, since we're merging values, the normal merging code will not work. After binding the first example, the file's exports have the following structure:

exports: {
  "export=": { valueDeclaration: module.exports = function ...
  "util": { valueDeclaration: module.exports.util = function ...
}

Then when the checker encounters var x = require(...), it looks up the file's exports and retrieves the "export=" entry.

This PR copies the other entries of exports over to export='s exports:

exports: {
  "export=": { 
    valueDeclaration: module.exports = function ..., 
    exports: {
      util: { valueDeclaration: ... }
    }
  },
  util: { valueDeclaration: ... }
}

This makes the export= symbol equivalent to the merged typescript in the second example.
Then when the checker asks for the type of this symbol, it can get the property assignments by looking at the exports of the symbol.

Since the same property can be assigned in both export= and property assignments, some unioning code is also required:

module.exports = { both: 1 }
module.exports.both = 'both: string | number'

In addition, if the module.exports = 1 (or any primitive type), then subsequent assignments will be ignored, so none of the property assignments should show up as exports either.

Fixes #22461

@sandersn sandersn requested a review from mhegazy April 6, 2018 19:38
@sandersn sandersn merged commit a7a01ea into master Apr 6, 2018
@sandersn sandersn deleted the js/allow-module-exports-with-exports-assignments branch April 6, 2018 20:04
mohsen1 pushed a commit to mohsen1/webpack that referenced this pull request Apr 11, 2018
sokra pushed a commit to webpack/webpack that referenced this pull request Apr 12, 2018
@microsoft microsoft locked and limited conversation to collaborators Jul 25, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants