Skip to content

Commit

Permalink
LPS-130359 Work around value visibility issue
Browse files Browse the repository at this point in the history
As noted in this issue:

    microsoft/TypeScript#37888

And specifically in my comment:

    microsoft/TypeScript#37888 (comment)

There is a problem in the TypeScript compiler involving indirect
references to unique Symbol types defined in projects connected by via
project references.

In short, `frontend-js-state-web` uses private `Symbol` instances as an
implementation detail, and these wind up in the type declaration files
as, eg:

    declare const ATOM: unique symbol;

So, when another project attempts to use a function like
`Liferay.State.atom()` that returns an object of a type involving that
symbol, TS may complain with:

    TS4023: Exported variable 'blah' has or is using name 'ATOM' from
    external module "path/to/State" but cannot be named.

The exact conditions required to trigger this are subtle, because using
`atom()` is working fine in places like the `frontend-js-react-web`
tests. My theory is that those work fine because the uses are all
internal, so TS doesn't need to emit any type information involving the
symbol.

But in the case of `item-selector-taglib`, which I am working on for the
purposes of this LPS, I have to export the atom, and that's when the
error pops up. Remove the `export` and there is no error. So the problem
is that TS has to emit a `.d.ts` file and it can't, because it can't
"see" the value of the symbol in the consuming module.

We can make the error go away like I am here in this commit, switching
from a symbol to a plain old string. These objects are read-only and
cannot be manipulated directly, so this is going to be fine, although I
did like the way the symbol keys were "invisible" from the outside.
  • Loading branch information
wincent authored and austinchiang committed Apr 19, 2021
1 parent ad2885b commit d6c5f4e
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 18 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,11 @@ import deepFreeze from './deepFreeze';

import type {Immutable} from './types';

const ATOM = Symbol('Liferay.State.ATOM');
const SELECTOR = Symbol('Liferay.State.SELECTOR');
// In the future, these should be Symbol(); see:
// https://github.com/microsoft/TypeScript/issues/37888

const ATOM = 'Liferay.State.ATOM';
const SELECTOR = 'Liferay.State.SELECTOR';

interface Getter {
<T>(atomOrSelector: Atom<T> | Selector<T>): Immutable<T>;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@
*/

import type {Immutable} from './types';
declare const ATOM: unique symbol;
declare const SELECTOR: unique symbol;
declare const ATOM = 'Liferay.State.ATOM';
declare const SELECTOR = 'Liferay.State.SELECTOR';
interface Getter {
<T>(atomOrSelector: Atom<T> | Selector<T>): Immutable<T>;
}
Expand All @@ -34,12 +34,12 @@ declare const State: {
readonly atoms: {
readonly default: Readonly<unknown>;
readonly key: string;
readonly [ATOM]: true;
readonly 'Liferay.State.ATOM': true;
}[];
readonly selectors: {
readonly deriveValue: (get: Getter) => unknown;
readonly key: string;
readonly [SELECTOR]: true;
readonly 'Liferay.State.SELECTOR': true;
}[];
};
reset(): void;
Expand Down Expand Up @@ -81,7 +81,7 @@ declare const State: {
selector: {
readonly deriveValue: (get: Getter) => T_2;
readonly key: string;
readonly [SELECTOR]: true;
readonly 'Liferay.State.SELECTOR': true;
},
seen: Set<Selector<unknown>>
): Immutable<T_2>;
Expand All @@ -106,7 +106,7 @@ declare const State: {
): {
readonly default: Immutable<T_3>;
readonly key: string;
readonly [ATOM]: true;
readonly 'Liferay.State.ATOM': true;
};

/**
Expand All @@ -119,12 +119,12 @@ declare const State: {
| {
readonly default: Immutable<T_4>;
readonly key: string;
readonly [ATOM]: true;
readonly 'Liferay.State.ATOM': true;
}
| {
readonly deriveValue: (get: Getter) => T_4;
readonly key: string;
readonly [SELECTOR]: true;
readonly 'Liferay.State.SELECTOR': true;
}
): Immutable<T_4>;

Expand All @@ -134,7 +134,7 @@ declare const State: {
readAtom<T_5>(atom: {
readonly default: Immutable<T_5>;
readonly key: string;
readonly [ATOM]: true;
readonly 'Liferay.State.ATOM': true;
}): Immutable<T_5>;

/**
Expand All @@ -143,7 +143,7 @@ declare const State: {
readSelector<T_6>(selector: {
readonly deriveValue: (get: Getter) => T_6;
readonly key: string;
readonly [SELECTOR]: true;
readonly 'Liferay.State.SELECTOR': true;
}): Immutable<T_6>;

/**
Expand All @@ -169,7 +169,7 @@ declare const State: {
key: string,
deriveValue: (get: Getter) => T_7
): {
readonly [SELECTOR]: true;
readonly 'Liferay.State.SELECTOR': true;
readonly deriveValue: (get: Getter) => T_7;
readonly key: string;
};
Expand All @@ -190,12 +190,12 @@ declare const State: {
| {
readonly default: Immutable<T_8>;
readonly key: string;
readonly [ATOM]: true;
readonly 'Liferay.State.ATOM': true;
}
| {
readonly deriveValue: (get: Getter) => T_8;
readonly key: string;
readonly [SELECTOR]: true;
readonly 'Liferay.State.SELECTOR': true;
},
callback: (value: Immutable<T_8>) => void
): {
Expand All @@ -217,12 +217,12 @@ declare const State: {
| {
readonly default: Immutable<T_9>;
readonly key: string;
readonly [ATOM]: true;
readonly 'Liferay.State.ATOM': true;
}
| {
readonly deriveValue: (get: Getter) => T_9;
readonly key: string;
readonly [SELECTOR]: true;
readonly 'Liferay.State.SELECTOR': true;
},
value: T_9
): void;
Expand All @@ -236,7 +236,7 @@ declare const State: {
atom: {
readonly default: Immutable<T_10>;
readonly key: string;
readonly [ATOM]: true;
readonly 'Liferay.State.ATOM': true;
},
value: T_10
): void;
Expand Down

0 comments on commit d6c5f4e

Please sign in to comment.