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

commonjs nested re-exports are broken #41422

Closed
sandersn opened this issue Nov 6, 2020 · 1 comment · Fixed by #41479
Closed

commonjs nested re-exports are broken #41422

sandersn opened this issue Nov 6, 2020 · 1 comment · Fixed by #41479
Assignees
Labels
Bug A bug in TypeScript Fix Available A PR has been opened for this issue High Priority

Comments

@sandersn
Copy link
Member

sandersn commented Nov 6, 2020

User tests compile with quite a few errors again. I haven't confirmed that this is the case on release-4.1, but I expect it is.

Follow-on to #40578, very likely a result of the PR that fixed that, plus the recent improvement to commonjs export aliasing.

@sandersn sandersn changed the title prettier imports are broken on master commonjs nested re-exports are broken Nov 10, 2020
@sandersn
Copy link
Member Author

Here's a repro. This seems unlikely to be common outside prettier, but we still need to get this working for 4.1:

// @filename: forever.js
const hardline = { type: "Hard" }
module.exports = { hardline }

// @filename: mod1.js
module.exports = {
  nested: require('./forever')
}

// @filename: test.js
const { hardline } = require('./mod1').nested
hardline

Notably, there's only an error on the second use of hardline in test.js: the first one is fine. The problem so far is that hardline resolves to module.exports = { hardline }. Previously, this wasn't an alias, and type checking proceeded to resolve to const hardline in forever.js. However, the new export alias is incorrectly not resolved, so the call to resolveSymbol retrieves an alias and issues an error since it expected a value.

sandersn added a commit that referenced this issue Nov 10, 2020
Fixes resolution of export aliases in the postfix-property-access case
of commonjs require:

```js
const { x } = require('./foo').nested
x
```

This program would previously fail if `x` was an export alias.

Fixes #41422
@sandersn sandersn added the Bug A bug in TypeScript label Nov 10, 2020
@sandersn sandersn added this to the TypeScript 4.1.1 milestone Nov 10, 2020
@typescript-bot typescript-bot added the Fix Available A PR has been opened for this issue label Nov 10, 2020
sandersn added a commit that referenced this issue Nov 10, 2020
Fixes resolution of export aliases in the postfix-property-access case
of commonjs require:

```js
const { x } = require('./foo').nested
x
```

This program would previously fail if `x` was an export alias.

Fixes #41422
andrewbranch pushed a commit to andrewbranch/TypeScript that referenced this issue Nov 25, 2020
Fixes resolution of export aliases in the postfix-property-access case
of commonjs require:

```js
const { x } = require('./foo').nested
x
```

This program would previously fail if `x` was an export alias.

Fixes microsoft#41422
andrewbranch added a commit that referenced this issue Dec 2, 2020
Fixes resolution of export aliases in the postfix-property-access case
of commonjs require:

```js
const { x } = require('./foo').nested
x
```

This program would previously fail if `x` was an export alias.

Fixes #41422

Co-authored-by: Nathan Shively-Sanders <293473+sandersn@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug A bug in TypeScript Fix Available A PR has been opened for this issue High Priority
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants