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

Unexpected TS4094 with the build parameter declaration: true #17293

Open
saschanaz opened this issue Jul 19, 2017 · 16 comments
Open

Unexpected TS4094 with the build parameter declaration: true #17293

saschanaz opened this issue Jul 19, 2017 · 16 comments
Labels
Bug A bug in TypeScript Domain: Declaration Emit The issue relates to the emission of d.ts files
Milestone

Comments

@saschanaz
Copy link
Contributor

TypeScript Version: 2.4.1

Code

var Foo = class Foo {
    private _message = "Hi";
    hi() { return this._message; }
}

tsc test.ts -t es2017 -d

Expected behavior: -d should not cause this message: test.ts(1,5): error TS4094: Property '_message' of exported class expression may not be private or protected.

Actual behavior: It does.

There should at least be a suppression parameter to ignore this error.

@mhegazy mhegazy added Bug A bug in TypeScript Domain: Declaration Emit The issue relates to the emission of d.ts files labels Aug 24, 2017
@weswigham weswigham added this to Not started in Rolling Work Tracking Aug 24, 2017
@mhegazy mhegazy modified the milestone: TypeScript 2.6 Sep 1, 2017
@tusharmath
Copy link

I am facing the same issue.

@mhegazy mhegazy modified the milestones: TypeScript 2.6, TypeScript 2.7 Oct 9, 2017
@yeliex
Copy link

yeliex commented Dec 6, 2017

any progress? @weswigham

@weswigham
Copy link
Member

Like many of our still-open declaration emit issues, fixing this would require significant changes to our declaration emit. Today, we'll try to emit the example in the OP as:

declare var Foo: {
  new (): {
    // Try to write type for `_message` here
    hi(): string;
  }
}

except since we're in an anonymous object literal type, there is no way to write a private or protected member, so instead we write a visibility error and refuse to generate declarations. The only "correct" thing to do to fix this is to generate an interface for Foo's instance type, that way member modifiers can be preserved. The issue with that is that generating an interface would change the publicly visible declarations for your file (namely, it would introduce one you never actually wrote, but was implicit in the usage of a class expression).

For now, the workaround is to explicitly give a type to your var Foo = /* ... */, ie var Foo: {new(): IFoo} = /* ... */, where you've defined IFoo.

The reason this issue and others like it (issues assigned to me in the category of declaration emit which I have not already created fixes for) are sitting for so long is because we're in the process of overhauling our declaration emitter to make it easier to generate things like these implicit interfaces (or imports, dependent variables, etc) without changing the public shape of your declaration file, and it's a long process to do the refactoring (generally speaking, big changes are difficult to review, and this is a series of big changes). We are replacing a 3-yr-old subsystem that contains >2000 LOC, countless edge-cases and deep connections to the type checker and emitter, after all. ;)

We are working on it, though. While I can't make any promises for this specific issue, we're hoping to have the prerequisite changes we want to make to make this possible done in the 2.7 timeframe.

@weswigham
Copy link
Member

weswigham commented Mar 22, 2018

@mhegazy The fix for this issue (and #17744) seem predicated on that idea that we could hoist these class expressions to a private name within a module and the reference them in the type definitions elsewhere. While this can work well inside modules or namespaces, in the global scope (like the example in the OP here), there's no way to do that hoisting without introducing a name that doesn't actually exist into the global scope (which is dangerous from an API perspective). Maybe we should consider a different approach? (anonymous class type nodes?)

@mhegazy
Copy link
Contributor

mhegazy commented Mar 28, 2018

(anonymous class type nodes?)

we did talk about this multiple times. do not see why not rely. we have parity between the other type nodes and their matching declarations.. except for class.

@rjamesnw
Copy link

rjamesnw commented Jun 3, 2018

I just ran into this as well using tsc 2.9.1. I'm exporting a class expression from a function with a couple protected members to be used in derived classes, and, although it compiles perfectly without declarations exported, exporting declarations causes it to fail miserably with this (and many other) errors. It's very annoying to have tsc compile perfectly, then have the declarations fail. :/

@emartinpi
Copy link

Anyone knows if there has been any progress with this?

@moccaplusplus
Copy link

I had the same issue. Using typescript 3.1.6. Had to comment out private keyword to avoid errors. (although it should logically be private).

/**
 * Day name in week. Text.
 * Examples:
 * 	Tuesday (EEEE or more)
 *  Tue (EEE or less)
 */
export const E = new class implements NodeFactory<DateTimeAccessor> {

  public readonly token = 'E';

  /* private */ readonly short = Node.of<DateTimeAccessor>((d, m) => m['weekdays.short.' + ISO8601Helper.dayOfWeek(d)]);

  /* private */ readonly full = Node.of<DateTimeAccessor>((d, m) => m['weekdays.full.' + ISO8601Helper.dayOfWeek(d)]);

  forOccurrence(n: number): Node<DateTimeAccessor> {
    return n < 4 ? this.short : this.full;
  }
};

tusharmath added a commit to tusharmath/ts-scheduler that referenced this issue Apr 15, 2019
@matthewadams
Copy link

matthewadams commented Aug 13, 2019

I just discovered this today, 13 Aug 2019. Reproable code is at matthewadams/typetrait@9b19118. Just clone & git checkout 9b19118aa33525b9d701fbee4f6ff8f223c1b6e8 && npm run build. @weswigham, can I get an update on when this will be fixed, please? OP was over two years ago. tsc --version yields Version 3.5.3.

matthewadams added a commit to matthewadams/typetrait that referenced this issue Aug 16, 2019
@weswigham
Copy link
Member

Hmm, honestly we need a proposal for class expression type nodes. You could also argue that inline type aliases with expanded modifiers in anonymous types would also do the trick. We've talked about the first before, informally, but not actually tossed together a proposal yet, since the only two issues that seem to require that level of expressivity and precision are, well, patterns like this and other similar mixin-type patterns. It's in the back of my mind, at least, if not a direct priority.

@matthewadams
Copy link

@weswigham Well, until it's addressed, my workaround is to devolve back to using _ (underscore) prefixes to indicate that a method is not public, which seems just dumb given TypeScript's access modifiers. Further, traits/mixins are, IMHO, the best (read: most practical) way to achieve composability and avoid brittle inheritance hierarchies, which is why I feel that it would do all of us good to have this fixed sooner rather than later.

@web-padawan
Copy link

This issue is important for us to write mixins that return classes with protected members.
So I summarised the workarounds we have to choose from at #17744 (comment)

While I like using Symbol, I would highly appreciate the fix for this issue (and #17744).

@trusktr
Copy link

trusktr commented Dec 21, 2019

Please see #35822

@sukima
Copy link

sukima commented Feb 17, 2023

Can someone explain in simple terms what is wrong with a class extending another class that has private properties/methods? This seems like very reasonable thing to do in OO design.

Specifically what is wrong with this:

class Foo {
  private foo = 'FOO';
}

function bar(value: string) {
  class Bar extends Foo {
    bar = value;
  }

  return new Bar();
}

@Marckon
Copy link

Marckon commented Feb 22, 2023

Can someone explain in simple terms what is wrong with a class extending another class that has private properties/methods? This seems like very reasonable thing to do in OO design.

Specifically what is wrong with this:

class Foo {
  private foo = 'FOO';
}

function bar(value: string) {
  class Bar extends Foo {
    bar = value;
  }

  return new Bar();
}

I don't know why but export Foo will eliminate this error

export class Foo {
  private foo = 'FOO';
}

function bar(value: string) {
  class Bar extends Foo {
    bar = value;
  }

  return new Bar();
}

@rjamesnw
Copy link

rjamesnw commented Feb 22, 2023

Can someone explain in simple terms what is wrong with a class extending another class that has private properties/methods? This seems like very reasonable thing to do in OO design.

Specifically what is wrong with this:

class Foo {
  private foo = 'FOO';
}

function bar(value: string) {
  class Bar extends Foo {
    bar = value;
  }

  return new Bar();
}

@sukima, see here: #30355 (comment)

phuhl added a commit to apparts-js/apparts-model that referenced this issue Jul 23, 2023
[ Unexpected TS4094 with the build parameter declaration: true #17293 ](microsoft/TypeScript#17293)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug A bug in TypeScript Domain: Declaration Emit The issue relates to the emission of d.ts files
Projects
Rolling Work Tracking
  
Not started
Development

No branches or pull requests