Explicitly typed prototype assignments are context sensitive #25688
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Follow up to #25619: Add the necessary code to type `prototype` correctly in prototype assignments so that code like `F.prototype = { ... }` properly makes the object literal context sensitive.
ghost
approved these changes
Jul 16, 2018
|
Travis says the linter failed; VSTS' output just says a log was missing (?). Edit: VSTS just failed to (1) wrap the line (2) show a visible scroll bar to indicate that the error was off the right side of the page. Same lint error though. |
sandersn
deleted the
js/explicitly-typed-prototype-assignments-are-context-sensitive
branch
sandersn
added a commit
that referenced
this pull request
Jul 20, 2018
* Revert "Revert "Explicitly typed special assignments are context sensitive (#25619)"" This reverts commit 16676f2. * Revert "Revert "Explicitly typed prototype assignments are context sensitive (#25688)"" This reverts commit ff8c30d. * Initial, wasteful, solution It burns a check flags. Probably necessary, but perhaps not. I haven't accepted baselines, but they are a bit questionable. I'm not sure the synthetic type is right, because I expected to see { "exports": typeof import("x") } but instead see { "x": typeof import("x") }. * Update some baselines * module.exports= always uses lhs type Conflicts between exports property assignments and exports assignments should get a union type instead of an error. * Fix lint and accept good user baselines * Add tests based on user tests. These currently fail. * Fix all but 1 of user test bugs found by typing module.exports Still quite messy and full of notes * Follow merged symbols+allow any object type This allows exports like `module.exports = new EE` to have properties added to them. Still messy, but I'm going to run user tests and look for regressions. * Update improved user baselines * Fix infinite recursion when checking module.exports * Fix bogus use-before-def error getExportSymbolOfValueSymbolIfExported should always merge its returned symbol, whether it's symbol.exportSymbol or just symbol. * Update user test baselines * Cleanup * More small cleanup * Bind `module` of `module.exports` as a special symbol Previously it was also special, but created during name resolution in the checker. It made sense when there was only one special symbol for all files, but now there is one `module` symbol per file.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Follow up to #25619: Add the necessary code to type
prototypecorrectly so that prototype assignments in code likeF.prototype = { ... }properly make the object literal context sensitive.Still doesn't fix
module.exports[property] assignments; the problem there is the typing of the symbolmodule, which is eitheranyor{ exports: any }.