firebase-functions icon indicating copy to clipboard operation
firebase-functions copied to clipboard

Unexpected side effect of `@hidden` to `@internal` migration - exposed definitons

Open merlinnot opened this issue 6 years ago • 4 comments

Related issues

None found.

[REQUIRED] Version info

node: v10.16.0

firebase-functions: v3.1.0

firebase-tools: v7.0.2

firebase-admin: v8.2.0

[REQUIRED] Test case

This will compile, whereas it shouldn't:

const error = new HttpsError('unknown', 'test-message');

console.log(error.toJSON());

[REQUIRED] Steps to reproduce

See above.

[REQUIRED] Expected behavior

The code provided in the example should not compile.

[REQUIRED] Actual behavior

The code provided in the example compiles. This is caused by a migration from @internal to @hidden to support TypeDoc. stripInternal option is still specified in tsconfig.release.json, but has no effect as no code has @internal annotations anymore.

Were you able to successfully deploy your functions?

Yes.

merlinnot avatar Jul 17 '19 10:07 merlinnot

I couldn't figure out how to label this issue, so I've labeled it for a human to triage. Hang tight.

google-oss-bot avatar Jul 17 '19 10:07 google-oss-bot

@merlinnot thanks for filing - I'm a bit confused by this one. Do you think this is a bug on the @hidden tag in TypeDoc? Do you think stripInternal is interfering here somehow?

thechenky avatar Jul 18 '19 18:07 thechenky

stripInternal was used to remove type definitions annotated with @internal from generated TypeScript type definitions - so for TypeScript users HttpsError.toJSON didn't exist, it was not present in type definitions.

After @internal was changed to @hidden, all of these properties are exposed in TypeScript type definitions - you can now access HttpsError.toJSON and it will be just fine, no compiler errors.

I'm wondering if we should use both @hidden and @internal:

  • @hidden so it's not shown in generated documentation
  • @internal so it's not shown in TypeScript type definitions (.d.ts files after running the build)

Alternatively we could simply not expose these (don't export, change public to private etc.), but since firebase-tools uses it, I believe it's not an option.

merlinnot avatar Jul 18 '19 19:07 merlinnot

Hmm interesting. I don't have anything against using both @hidden and @internal. Ideally we would stick to one - I know there's a plugin for making TypeDoc work with @internal but after playing around with it I decided it was easier to change to @hidden, but I can poke around more on that. @merlinnot do you have experience in the TypeDoc area?

thechenky avatar Jul 18 '19 23:07 thechenky

Update from the future - I think this is an interesting enhancement, but not worth tackling atm. Closing the issue to declutter our backlog.

taeold avatar Dec 28 '22 20:12 taeold