GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — Private named instance fields by mheiber · Pull Request #30829 · microsoft/TypeScript

Conversation

@mheiber

@mheiber Max Heiber (mheiber) commented Apr 9, 2019 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

Private-Named Instance Fields

This PR implements the tc39 class fields proposal for TypeScript. It includes:

  • parse and check private-named fields, methods, and accessors
  • displayprivate names in the language server
  • transform private-named instance fields

PR merge checklist

  • BB: incorporate remaining feedback
  • BB: add multiple @targets to conformance tests esp this one
  • MS: reviewer 1 👍 remaining feedback
  • MS: reviewer 2 👍 remaining feedback
  • BB: squash and rebase
  • MS: run rw test suite
  • merge companion PR to tslib

Example:

ts

class Greeter {
    #name: string;
    constructor(name: string) {
        this.#name = name;
    }
    greet() {
        console.log(`hello ${this.#name}`);
    }
}

const greeter = new Greeter("world");
console.log(greeter);                 // logs 'Greeter {}'
console.log(Object.keys(greeter));    // logs '[]'
greeter.greet();                      // logs 'hello world'

js

var _classPrivateFieldSet = function (receiver, privateMap, value) { if (!privateMap.has(receiver)) { throw new TypeError("attempted to set private field on non-instance"); } privateMap.set(receiver, value); return value; };
var _classPrivateFieldGet = function (receiver, privateMap) { if (!privateMap.has(receiver)) { throw new TypeError("attempted to get private field on non-instance"); } return privateMap.get(receiver); };
var _nameWeakMap_1;
class Greeter {
    constructor(name) {
        _nameWeakMap_1.set(this, void 0);
        _classPrivateFieldSet(this, _nameWeakMap_1, name);
    }
    greet() {
        console.log(`hello ${_classPrivateFieldGet(this, _nameWeakMap_1)}`);
    }
}
_nameWeakMap_1 = new WeakMap();
const greeter = new Greeter("world");
console.log(greeter); // logs 'Greeter {}'
console.log(Object.keys(greeter)); // logs '[]'
greeter.greet(); // logs 'hello world'
  • Related:
    • Chrome 74 announcement (includes private class fields)
    • tc39 class fields proposal

This PR lead to the following related work by the team:

  • Babel issues reported:

    • Parser allows spaces between # and identifier babel/babel#8225
    • @babel/plugin-proposal-class properties: .name incorrect for private fields babel/babel#10175
    • Invalid code emitted for computed properties babel/babel#8882
  • V8 issues reported:

    • https://bugs.chromium.org/p/v8/issues/detail?id=9560
    • https://bugs.chromium.org/p/chromium/issues/detail?id=1014458
    • https://bugs.chromium.org/p/v8/issues/detail?id=8144#c4
    • https://bugs.chromium.org/p/chromium/issues/detail?id=982267#c10
  • Related TS PRs:

    • Fix block-scoped capturing by class elements inside iteration statements #28708
    • Move class property transformation into new transformer file #30467

This PR includes work by the following engineers:

  • Joey Watts Joey Watts (@joeywatts) (Bloomberg): private fields transformation
  • Max Heiber Max Heiber (@mheiber) (Bloomberg): parser and checker, IR, pr review
  • Michael Gunter Michael Gunter (@Neuroboy23) (Bloomberg): pr review
  • Rob Palmer Rob Palmer (@robpalme) (Bloomberg): advice, spec expertise, pr review
  • Ravi Amin (Bloomberg): pr review
  • Jaideep Bhoosreddy Jaideep Bhoosreddy (@jbhoosreddy), Michael Molisani Michael Molisani (@molisani), Ravi Amin (Bloomberg)
  • Ron Buckton Ron Buckton (@rbuckton) (Microsoft): designed the checker implementation, lots of pr review
  • Daniel Rosenwasser Daniel Rosenwasser (@DanielRosenwasser) (Microsoft): declaration file emit, pr review
  • Nathan Shively-Sanders Nathan Shively-Sanders (@sandersn) (Microsoft): lots of pr review
  • Thanks for discussion with many more engineers, including Wesley Wigham (@weswigham) , Ryan Cavanaugh (@RyanCavanaugh) , Anders Hejlsberg (@ahejlsberg) , Kevin Smith (@zenparsing), Daniel Ehrenberg (@littledan), and Tim McClure (@tim-mc)

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

trotyl, mihailik, RichiCoder1, chriskrycho, swarnimarun, saschanaz, DanielRamosAcosta, martinmcwhorter, a1essar, SalvatorePreviti, and 66 more reacted with thumbs up emoji monfa-red, dimitropoulos, diabolicle, corbane, chiqui3d, chriskuech, nulladdict, demensky, styfle, quadband, and 12 more reacted with thumbs down emoji trotyl, aduh95, 0kku, jbhoosreddy, MattiasBuelens, motss, alitaheri, dnalborczyk, styfle, tim-mc, and 2 more reacted with hooray emoji trotyl, mihailik, johnnyreilly, aduh95, SalvatorePreviti, G-Rath, 0kku, jbhoosreddy, JunichiSugiura, francesco-agnoletto, and 15 more reacted with heart emoji v-checha and riyadshauk reacted with rocket emoji houfio, sod, johnnyreilly, motss, and amitbeck reacted with eyes emoji
All reactions
  • 👍 76 reactions
  • 👎 22 reactions
  • 🎉 12 reactions
  • ❤️ 25 reactions
  • 🚀 2 reactions
  • 👀 5 reactions
@msftclas

Microsoft Contribution License Agreements (msftclas) commented Apr 9, 2019 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown

CLA assistant check
All CLA requirements met.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@mheiber
Max Heiber (mheiber) force-pushed the private-named-instance-fields branch 2 times, most recently from b242db5 to f2c5233 Compare April 9, 2019 12:33
@mihailik

mihailik commented Apr 15, 2019

Copy link
Copy Markdown
Contributor

Amazing achievement!

Are the helpers _classPrivateFieldGet/_classPrivateFieldSet treated in the same way as __extends, __awaiter, __generator?

(that would include modifications to tslib)

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@mheiber

Max Heiber (mheiber) commented Apr 17, 2019

Copy link
Copy Markdown
Contributor Author

mihailik thanks! As far as I know, the helpers work the same way. Is any additional work required to get the helpers into tslib? I was hoping the syncing was semi-automated.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@weswigham

Wesley Wigham (weswigham) commented Apr 17, 2019

Copy link
Copy Markdown
Member

I was hoping the syncing was semi-automated.

Sadly, they are not. A paired PR against tslib with the finalized helpers will be needed.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@Neuroboy23

Michael Gunter (Neuroboy23) commented Apr 17, 2019

Copy link
Copy Markdown

Wesley Wigham (@weswigham): I have asked our IP team to set up a fork of the tslib repo.

Joey Watts (@joeywatts) and Max Heiber (@mheiber): Will you take a look at this other repo? I think it will be an easy mod.

shyce reacted with thumbs up emoji weswigham reacted with heart emoji
All reactions
  • 👍 1 reaction
  • ❤️ 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@RyanCavanaugh Ryan Cavanaugh (RyanCavanaugh) assigned Daniel Rosenwasser (DanielRosenwasser) Apr 30, 2019
@weswigham Wesley Wigham (weswigham) mentioned this pull request May 8, 2019
Overriding an accessor with a property causes runtime error when emitting ESNExt #31225
Closed
@mbrowne Matt Browne (mbrowne) mentioned this pull request May 15, 2019
Implement private fields proposal #9950
Closed
@DanielRosenwasser Daniel Rosenwasser (DanielRosenwasser) mentioned this pull request May 29, 2019
TypeScript 3.6 Iteration Plan #31639
Closed
@G-Rath

Gareth Jones (G-Rath) commented May 30, 2019 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown

Forgive me if this is obvious, but I've look around on the issues and couldn't see this covered anywhere:

Is there any particular reason why we can't do private #bar = 3;?
(In case its not clear, I mean this purely as a meaningless sugar that would never make it into compiled code)

Would there be any chance of getting that syntax supported?

In my eyes it'd give us the best of both worlds, as then all class properties can maintain their indentation level, along with making refactoring easy for people like me who prefix all our private properties with _.

I mean you could pretty much just use a global find-and-replace using /private #/ regex, since # is only valid for private class properties - the only issue with this I can think of would be with strings.

Also, could someone confirm for me if its planned for TS somewhere down the line to transform private to #? (I suspect it is, but again there's a lot of heavy comments, PRs, & issues on the matter, so it'd be nice to have a simple one-liner comment about it if possible).

Sovgut, matrunchyk, LifeIsStrange, ems-ja, lsagetlethias, dfernandeza, dlOuOlb, tommytroylin, kokushkin, and 1valdis reacted with thumbs up emoji trotyl reacted with thumbs down emoji
All reactions
  • 👍 10 reactions
  • 👎 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@ljharb

Jordan Harband (ljharb) commented May 30, 2019

Copy link
Copy Markdown
Contributor

Gareth Jones (@G-Rath) no, no chance of it being supported by JS itself (see the FAQ). I suspect/hope TS would not be likely to support non-type-related syntax that isn't standard to JS.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@G-Rath

Gareth Jones (G-Rath) commented May 30, 2019

Copy link
Copy Markdown

Jordan Harband (@ljharb) fair enough - so then I assume TS is aiming to deprecate & remove the private keyword? (over time of course)

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@robpalme

Rob Palmer (robpalme) commented May 30, 2019

Copy link
Copy Markdown

Gareth Jones (@G-Rath) Your question on the future of TypeScript's keyword private is one that many people will have, and may provoke more discussion. Would you like to raise it as a new top-level issue?

levenleven reacted with thumbs up emoji G-Rath and datatypevoid reacted with heart emoji
All reactions
  • 👍 1 reaction
  • ❤️ 2 reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@G-Rath Gareth Jones (G-Rath) mentioned this pull request May 30, 2019
The future of the "private" keyword #31670
Closed
@G-Rath

Gareth Jones (G-Rath) commented May 30, 2019

Copy link
Copy Markdown

Rob Palmer (@robpalme) more than happy to - done via #31670

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@septs

septs (septs) commented May 31, 2019

Copy link
Copy Markdown
Contributor

"private field" decorator "private" only?

class Sample {
    private #field: number = 1;
}
ExE-Boss, ems-ja, jerry-TangHao, and 17621152239 reacted with thumbs up emoji jhpratt and fkajerry reacted with thumbs down emoji G-Rath, septs, RyanCavanaugh, MichalLytek, AsukaSong, and hasezoey reacted with confused emoji
All reactions
  • 👍 4 reactions
  • 👎 2 reactions
  • 😕 6 reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@robpalme

Rob Palmer (robpalme) commented Jun 18, 2019

Copy link
Copy Markdown

Ron Buckton (@rbuckton) has re-landed the refactoring of class properties. The next step is to rebase this PR.

mheiber reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@Aqours

L&H (Aqours) commented Jun 21, 2019

Copy link
Copy Markdown

Does private name use same private fields transformation of #name?

michahell and jerry-TangHao reacted with thumbs up emoji
All reactions
  • 👍 2 reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@DanielRosenwasser

Daniel Rosenwasser (DanielRosenwasser) commented Jun 21, 2019

Copy link
Copy Markdown
Member

L&H (@Aqours) no, private is meant to be strictly design-time.

littledan reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@mheiber

Max Heiber (mheiber) commented Jun 21, 2019

Copy link
Copy Markdown
Contributor Author

The rebase is in progress and we're expecting to update soon!

RobertWHurst reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@mheiber

Max Heiber (mheiber) commented Jun 21, 2019 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor Author

I noticed an issue with our Language Service changes. Type inference and red underlines work correctly, but autocomplete is borked.

Steps to reproduce:

In a class with private field #foo#, start typing this.#foo

Actual behavior:

langservice

Autocomplete completes incorrectly (see screenshot). It is actually completing with secret variables we use in the transformation

Expected behavior:

Autocomplete completes as #foo.

Advice welcome on how to fix autocomplete in this PR!

gugadev reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@DanielRosenwasser

Daniel Rosenwasser (DanielRosenwasser) commented Jun 24, 2019

Copy link
Copy Markdown
Member

That is actually really weird. Are you sure that it's not just picking up the output .js files? Are you able to reproduce in a fourslash test?

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@mheiber
Max Heiber (mheiber) force-pushed the private-named-instance-fields branch from f2c5233 to a9cb10e Compare June 25, 2019 08:47
@mheiber

Max Heiber (mheiber) commented Jun 25, 2019

Copy link
Copy Markdown
Contributor Author

Thanks for your suggestion, Daniel Rosenwasser (@DanielRosenwasser): the completions were picking up the JS output. I added an export to let TS know the file is a module, and no longer see weird completions.
Unfortunately, I also don't see the desired completion, which is this.#foo:

image

DanielRosenwasser reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@joeywatts
Joey Watts (joeywatts) force-pushed the private-named-instance-fields branch from a9cb10e to 36a1648 Compare June 26, 2019 19:31
@mheiber
Max Heiber (mheiber) force-pushed the private-named-instance-fields branch from 36a1648 to d2adc3c Compare June 27, 2019 10:31
@joeywatts
Joey Watts (joeywatts) force-pushed the private-named-instance-fields branch 2 times, most recently from 71c384a to afcc88c Compare June 27, 2019 21:23
@mheiber

Max Heiber (mheiber) commented Dec 31, 2019 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor Author

fwiw, I agree that the hack is a bad idea. Wouldn't want the hack to be a secret, though, since it's good to know that the privacy of the transformed code is not absolute, barring control over the WeakMap prototype. Daniel Ehrenberg (@littledan), are there other privacy hills privacy holes, as well?

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@littledan

Daniel Ehrenberg (littledan) commented Dec 31, 2019

Copy link
Copy Markdown

Max Heiber (@mheiber) What do you mean by privacy hills?

mheiber reacted with laugh emoji
All reactions
  • 😄 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@mheiber

Max Heiber (mheiber) commented Dec 31, 2019 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor Author

I meant "privacy holes." Autocorrect got me!

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@littledan

Daniel Ehrenberg (littledan) commented Dec 31, 2019 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown

I'm not aware of any privacy holes in the main design. There may be other hacks that just work on this transform, though.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@Jamesernator

James Browning (Jamesernator) commented Jan 2, 2020 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown

We have no plans to implement #-private parameter properties.

The feature is pretty popular, I imagine a lot of places won't adopt private fields primarily for this reason.

Has a similar feature been considered for proposing at TC39? e.g.:

class Point {
    constructor(#x: number, #y: number) {}
}

And for public fields maybe allow using this.prop as a parameter (or destructured parameter) e.g.:

class Image {
  constructor(
    this.data: ImageData, // Simple parameter
    {
      height: this.height,
      width: this.width /* in destructuring */
    }: { width: number, height: number },
  ) {}
}
ctsstc and danbord reacted with thumbs up emoji
All reactions
  • 👍 2 reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@avonwyss

Arsène von Wyss (avonwyss) commented Jan 2, 2020

Copy link
Copy Markdown

Given that the __classPrivateFieldGet and __classPrivateFieldSet could be customized in the way they work by implementing them differently and importing that customized tslib, wouldn't it make sense to also put the _nameWeakMap_1 = new WeakMap(); map initialization into its own tslib helper function?

This would allow to implement a version which also works without WeakMap support (e.g. IE <= 10), or for anyone who wants to use a different mechanism, such as with non-enumerable properties or Symbol-based private fields (knowing that these are not private as per tc39 proposal, but may perform better).

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@mbrowne

Matt Browne (mbrowne) commented Jan 3, 2020

Copy link
Copy Markdown

In case anyone else is wondering the same thing I was, it looks like the first production release to include this will be version 3.8, scheduled for sometime in February:
https://github.com/microsoft/TypeScript/wiki/Roadmap#38-february-2020

kohlmannj, awkaiser, and aloisbarreras reacted with thumbs up emoji kokushkin reacted with thumbs down emoji hybrist reacted with hooray emoji
All reactions
  • 👍 3 reactions
  • 👎 1 reaction
  • 🎉 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Sheetal Nandi (sheetalkamat) added a commit to microsoft/TypeScript-TmLanguage that referenced this pull request Jan 8, 2020
@sheetalkamat
Private fields corresponding to microsoft/TypeScript#30829
f065e7e
@kokushkin

Nikolai Kulikov (kokushkin) commented Jan 23, 2020

Copy link
Copy Markdown

This looks ridiculous and disrupt developer experience with the language, especially for newers) Please, don't do it. This is feature for Typescript 4, not for 3.8, where we can break backward compatibility. Or, at least either "private" or "#" must be deleted in Typescript 4.

kokushkin, hax, colonelchlorine, and TeoTN reacted with thumbs up emoji ljharb, hybrist, pyrocat101, and ShlomiAltostra reacted with thumbs down emoji aduh95 and matrunchyk reacted with laugh emoji kokushkin reacted with confused emoji
All reactions
  • 👍 4 reactions
  • 👎 4 reactions
  • 😄 2 reactions
  • 😕 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@mbrowne

Matt Browne (mbrowne) commented Jan 24, 2020

Copy link
Copy Markdown

Nikolai Kulikov (@kokushkin) Obviously # should not be dropped in TypeScript 4 since it's already an ECMAScript standard (still stage 3, but advancing to stage 4 is just a formality at this point in the process). And I think dropping private would be premature since it would break backward compatibility and there is currently no other way to create compile-time only private properties (or at least properties that are accessible somehow outside the class, e.g. symbols). Anyway, this PR isn't the best place for these discussions...just thought I would briefly give my two cents in response to this.

hybrist reacted with thumbs up emoji kokushkin reacted with thumbs down emoji
All reactions
  • 👍 1 reaction
  • 👎 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@hax

HE Shi-Jun (hax) commented Jan 27, 2020

Copy link
Copy Markdown

since it's already an ECMAScript standard

This is not true. See my previous comment.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@mbrowne

Matt Browne (mbrowne) commented Jan 27, 2020

Copy link
Copy Markdown

HE Shi-Jun (@hax) You're right, I didn't mean to mislead anyone, just should have written more carefully... I was trying to emphasize that this isn't just an early-stage proposal that TypeScript decided to implement on their own. I should have said it's a proposed standard that's very near the final stage of the standardization process, with private fields now enabled by default in Chrome, Firefox, and node.

matrunchyk, hybrist, and Sominemo reacted with thumbs up emoji
All reactions
  • 👍 3 reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@hybrist

Jan Olaf Martin (hybrist) commented Jan 27, 2020 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown

with private fields now enabled by default in Chrome, Firefox, and node.

Maybe you meant s/Firefix/Edge/? Firefox has public instance fields but no real support for private fields yet. Though it looks like it's just a matter of time until they get to it: https://bugzilla.mozilla.org/show_bug.cgi?id=1562054. But >50% of web users have private fields enabled so your overall point stands I think. :)

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@mbrowne

Matt Browne (mbrowne) commented Jan 27, 2020

Copy link
Copy Markdown

@jkrems Ah yes, thanks for the correction. The public fields part of the proposal is enabled by default in Firefox...next step will probably be to add private fields behind a flag. Implementation status is periodically updated here:
https://github.com/tc39/proposal-class-fields#implementations

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@Jolg42 Joël Galeran (Jolg42) mentioned this pull request Jan 30, 2020
Hide top-level client properties (as they are visible via JS code intellisense) prisma/prisma-client-js#421
Closed
@JoshuaKGoldberg Josh Ghoulberg 👻 (JoshuaKGoldberg) mentioned this pull request Feb 15, 2020
Allowed non-this, non-super code before super call in derived classes with property initializers #29374
Merged
@danielearwicker

Daniel Earwicker (danielearwicker) commented Feb 20, 2020

Copy link
Copy Markdown

A question about the downlevel emit - to use this feature I have to target ES2015. But that will mean classes remain as classes, which breaks IE11. And yet IE11 supports WeakMap to some extent. And I find that TS does actually produce what looks like a good emit private fields for ES5, but gives me an error message.

Is IE11's WeakMap good enough for this usage? It returns undefined from set but the TS emit here doesn't care about that.

If I could suppress the error somehow... #29950

cdata, ExE-Boss, justinfagnani, pyrocat101, and UltiRequiem reacted with thumbs up emoji
All reactions
  • 👍 5 reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

kdesysadmin pushed a commit to KDE/syntax-highlighting that referenced this pull request Feb 29, 2020
@nibags @christoph-cullmann
TypeScript: add private fields and type-only imports/exports, and som…
0daf938
…e fixes

Summary:
* Add private-named instance fields. Ex: `x.#name;`. [1]
* Support type-only imports and exports. [2]
* Improve the detection of conditional expressions `a ? b : c`. Allow multiple lines.
* Add rules of round brackets `()` to correct the highlighting of pairs of brackets. [3]

[1] microsoft/TypeScript#30829
[2] microsoft/TypeScript#35200
[3] https://unix.stackexchange.com/questions/527268/kate-18-12-3-no-longer-shows-matching-parenthesis-for-typescript

Reviewers: #framework_syntax_highlighting, dhaumann, cullmann

Reviewed By: #framework_syntax_highlighting, cullmann

Subscribers: kwrite-devel, kde-frameworks-devel

Tags: #kate, #frameworks

Differential Revision: https://phabricator.kde.org/D27692
@wincent Greg Hurrell (wincent) mentioned this pull request Mar 10, 2020
feat: Prepare a new snapshot version to integrate in Portal's build liferay/liferay-js-toolkit#542
Merged
@trusktr

Joe Pea (trusktr) commented May 14, 2020 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

The implementation could use a single WeakMap per module, instead of one WeakMap per property. It think it may be more efficient.

Maybe I missed it: is there a specific reason it needs to be one WM per property?

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@ljharb

Jordan Harband (ljharb) commented May 14, 2020

Copy link
Copy Markdown
Contributor

At the least you’d need one for statics, and one for instances - but actually, it wouldn’t necessarily be more efficient, since you’d need a containing object to look up the individual fields in - and for functions, you’d have to be very careful not to expose the containing object as the receiver.

ExE-Boss reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@trusktr

Joe Pea (trusktr) commented May 14, 2020 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

Ah, right! I take that back, because for each instance, we'd need that accompanying object, which means O(n) instead of O(1) mem use for storage containers for the key-value pairs, where n is number of class instances and 1 is the constant number of defined private fields. I overlooked that.

ljharb and ExE-Boss reacted with thumbs up emoji
All reactions
  • 👍 2 reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@mbrowne

Matt Browne (mbrowne) commented May 22, 2020

Copy link
Copy Markdown

Joe Pea (@trusktr) FYI, there's also this issue: https://github.com/tc39/proposal-class-fields/blob/master/PRIVATE_SYNTAX_FAQ.md#how-can-you-model-encapsulation-using-weakmaps.

Babel works around this by creating a separate WeakMap for each field.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@steffenagger Steffen Agger (steffenagger) mentioned this pull request Jun 23, 2020
Merge Realm JS and Realm Web types realm/realm-js#2950
Merged
@j-oliveras Jordi Oliveras Rovira (j-oliveras) mentioned this pull request Jul 7, 2020
Provide a modifier to allow classes to protect property state while allowing mutations/changes within the class #39461
Closed
5 tasks done
@dragomirtitian Titian Cernicova-Dragomir (dragomirtitian) mentioned this pull request Feb 3, 2021
ES private class elements #42458
Merged
3 tasks
@TimvdLippe Tim van der Lippe (TimvdLippe) mentioned this pull request Jun 21, 2021
Allow private class field notation in parameter properties #44683
Closed
5 tasks done
@github-actions github-actions Bot mentioned this pull request Aug 14, 2022
Remove span tag in title and unnecessary TOC in Ko microsoft/TypeScript-Website-Localizations#164
Merged
@brandonbothell Brandon Bothell (brandonbothell) mentioned this pull request Jun 23, 2023
Switch private methods and fields to #truePrivate brandonbothell/popyt#511
Closed
@microsoft Microsoft (microsoft) locked as resolved and limited conversation to collaborators Oct 21, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@DanielRosenwasser Daniel Rosenwasser (DanielRosenwasser) DanielRosenwasser requested changes
@sheetalkamat Sheetal Nandi (sheetalkamat) sheetalkamat left review comments
@sandersn Nathan Shively-Sanders (sandersn) sandersn approved these changes
+4 more reviewers
@ljharb Jordan Harband (ljharb) ljharb left review comments
@joeywatts Joey Watts (joeywatts) joeywatts left review comments
@rbuckton Ron Buckton (rbuckton) rbuckton requested changes
@Jamesernator James Browning (Jamesernator) Jamesernator left review comments
Reviewers whose approvals may not affect merge requirements

Uh oh!

There was an error while loading. Please reload this page.

Assignees

@sandersn Nathan Shively-Sanders (sandersn)

@rbuckton Ron Buckton (rbuckton)

Labels

Update Docs on Next Release Indicates that this PR affects docs

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

20 participants

@mheiber @msftclas @mihailik @weswigham @Neuroboy23 @G-Rath @ljharb @robpalme @septs @Aqours @DanielRosenwasser @sandersn @rbuckton @joeywatts @aduh95 @orta @typescript-bot @ExE-Boss @nicolo-ribaudo @gugadev
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.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.