Skip to content

Revert "Revert "Support declaration emit for late bound element acces… #37035

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

Closed

Conversation

weswigham
Copy link
Member

…ses assigned to functions in both TS and JS (#36593)" (#37034)"

This reverts commit 4d5464e.

Fixes #33569

…ses assigned to functions in both TS and JS (microsoft#36593)" (microsoft#37034)"

This reverts commit 4d5464e.
@weswigham
Copy link
Member Author

@typescript-bot user test this so I can keep around a log of the enhanced-resolve failure until I isolate the issue.

@typescript-bot
Copy link
Collaborator

typescript-bot commented Feb 26, 2020

Heya @weswigham, I've started to run the parallelized community code test suite on this PR at a24659d. You can monitor the build here.

@typescript-bot
Copy link
Collaborator

The user suite test run you requested has finished and failed. I've opened a PR with the baseline diff from master.

@sandersn
Copy link
Member

@weswigham is this meant for merging? Or is it waiting for additional fixes after you figure out the enhanced-resolve failure?

@sandersn sandersn added the For Milestone Bug PRs that fix a bug with a specific milestone label Mar 12, 2020
@weswigham
Copy link
Member Author

Waiting for an additional fix~

@sandersn
Copy link
Member

What is the additional fix?

@weswigham
Copy link
Member Author

Mmmmm, sorry, I haven't gotten back to it yet; but I'm pretty sure I need to rethink how the cjs merge/late bound exports/normally bound exports merge sequence happens in its entirety. What this change does right now is ultimately a kind of hack; but the reason I tried that is because the alternative is essentially scrapping the current "cjs module symbol merge" model in the checker. It's not great, but I didn't really want to undertake redoing it to better integrate with normal symbol member resolution if I could help it. The enhanced-resolve break just proved that this hack wasn't robust enough for the real world, so the more complete work (to make it so "cjsy" exports are looked up either by normal export or late bound export resolution without overwriting the symbol) is required, probably.

@sandersn
Copy link
Member

sandersn commented Apr 1, 2020

This PR hasn't seen any activity for quite a while, so I'm going to close it to keep the number of open PRs manageable. Feel free to open a fresh PR or continue the discussion here.

@sandersn sandersn closed this Apr 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Author: Team For Milestone Bug PRs that fix a bug with a specific milestone
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Handle element access expressions in JS/TS with late-bound names
3 participants