fix(@lexical/utils): fix #5918 by re-exporting shared/* constants with explicit types

[etrepum]

Quick hotfix to re-export the shared/* constants with an explicit export const X: boolean = X_; (where the original constant is imported renamed as X_). This ensures that TypeScript's output does not include any imports to the source of X_.

I'm not sure if there's a better way to get typescript to inline types from private modules, but I wanted to get this hotfix out to fix the regression introduced by #5831. I had time to do a bit more research and add a validation step so that this doesn't happen again. It seems that a more ergonomic solution to this class of problems would be to use api-extractor (#5921).

fixes #5918

[vercel[bot]]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
lexical	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Apr 18, 2024 8:19pm
lexical-playground	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Apr 18, 2024 8:19pm

[etrepum]

When the tests fail it looks like this:

[StyleT]

Hi! Shouldn't it be possible for the compiler to simply bundle shared/* files? This is what tsup does for example..

I guess proper hotfix here is to rollback #5831 and do patch release

[etrepum]

The compiler is creating the shared files in .ts-temp/shared but in order for them to be useful they would need to be copied somewhere inside the packages that depend on them and then the imports would need to be rewritten to use relative rather than absolute paths (e.g. ./_shared/x instead of shared/x). I am not aware of any options to the typescript compiler to get it to do what we want for this situation, I think we do need to bring in a build tool like api-extractor or maybe tsup to do this for us (short of doing something bespoke).

[StyleT]

The compiler is creating the shared files in .ts-temp/shared but in order for them to be useful they would need to be copied somewhere inside the packages that depend on them and then the imports would need to be rewritten to use relative rather than absolute paths (e.g. ./_shared/x instead of shared/x). I am not aware of any options to the typescript compiler to get it to do what we want for this situation, I think we do need to bring in a build tool like api-extractor or maybe tsup to do this for us (short of doing something bespoke).

You're right. tsc can't do this. I would propose to replace it with proper type declarations bundler.

https://github.com/microsoft/TypeScript/issues/4433 presents some good ideas on how one might accomplish that with community-maintained tools, including API Extractor, rollup-plugin-dts, and tsup.

So instead of trying to rush suboptimal solution here - rollback & switch to one of the better options instead.

But I don't have a strong position here if someone wants to merge it.

[StyleT]

Update here. So tsup works kinda out of the box here... Just do:

# Install tsup for monorepo
$ cd packages/lexical-utils
$ npx tsup ./src/index.ts --dts-only
# Profit!

I can probably fix it later on. Or @etrepum you can do it. How:

1. Add "types-build" command to every package in monorepo. Because tsup expects entrypoint defined explicitly
2. Call it from build script

[etrepum]

I could switch the build script to use tsup in #5876 instead of calling tsc, looks like it will be fairly straightforward. I think either way it makes sense to have the sanity check that I added in the second commit https://github.com/facebook/lexical/pull/5920/commits/bae93d3a90101165c79a3e40de5448a89f42f81b

[etrepum]

tsup is a bit slower than I expected, each package takes maybe two seconds to process (and they all have to be handled separately due to the way tsup works, doing it in-process doesn't seem to change much).

I think the output is mostly ok, but there are some unintended consequences:

• It repeats the copyright header a lot (once per source file that was rolled up)
• It will bring in possibly unwanted declarations, like this one:

  declare global {
  interface Document {
      documentMode?: unknown;
  }
  interface Window {
      MSStream?: unknown;
  }
  }

relevant tsup config

```typescript async function buildTSDeclarationFiles() { for (const pkg of packagesManager.getPublicPackages()) { const cwd = process.cwd(); try { process.chdir(pkg.resolve()); await tsup.build({ dts: {only: true}, entry: pkg .getExportedNpmModuleEntries() .map((entry) => `src/${entry.sourceFileName}`), noExternal: [/^shared($|\/)/], silent: true, tsconfig: '../../tsconfig.build.json', }); } finally { process.chdir(cwd); } } } ```

packages/lexical-utils/dist/index.d.ts output

```typescript import { LexicalEditor, LexicalNode, ElementNode, Klass, EditorState } from 'lexical'; export { $splitNode, isHTMLAnchorElement, isHTMLElement } from 'lexical'; /** * Copyright (c) Meta Platforms, Inc. and affiliates. * * This source code is licensed under the MIT license found in the * LICENSE file in the root directory of this source tree. * */ declare function markSelection(editor: LexicalEditor, onReposition?: (node: Array) => void): () => void; /** * Copyright (c) Meta Platforms, Inc. and affiliates. * * This source code is licensed under the MIT license found in the * LICENSE file in the root directory of this source tree. * */ type Func = () => void; /** * Returns a function that will execute all functions passed when called. It is generally used * to register multiple lexical listeners and then tear them down with a single function call, such * as React's useEffect hook. * @example * ```ts * useEffect(() => { * return mergeRegister( * editor.registerCommand(...registerCommand1 logic), * editor.registerCommand(...registerCommand2 logic), * editor.registerCommand(...registerCommand3 logic) * ) * }, [editor]) * ``` * In this case, useEffect is returning the function returned by mergeRegister as a cleanup * function to be executed after either the useEffect runs again (due to one of its dependencies * updating) or the component it resides in unmounts. * Note the functions don't neccesarily need to be in an array as all arguements * are considered to be the func argument and spread from there. * @param func - An array of functions meant to be executed by the returned function. * @returns the function which executes all the passed register command functions. */ declare function mergeRegister(...func: Array): () => void; /** * Copyright (c) Meta Platforms, Inc. and affiliates. * * This source code is licensed under the MIT license found in the * LICENSE file in the root directory of this source tree. * */ declare function positionNodeOnRange(editor: LexicalEditor, range: Range, onReposition: (node: Array) => void): () => void; /** * Copyright (c) Meta Platforms, Inc. and affiliates. * * This source code is licensed under the MIT license found in the * LICENSE file in the root directory of this source tree. * */ declare const CAN_USE_DOM: boolean; /** * Copyright (c) Meta Platforms, Inc. and affiliates. * * This source code is licensed under the MIT license found in the * LICENSE file in the root directory of this source tree. * */ declare global { interface Document { documentMode?: unknown; } interface Window { MSStream?: unknown; } } declare const IS_APPLE: boolean; declare const IS_FIREFOX: boolean; declare const CAN_USE_BEFORE_INPUT: boolean; declare const IS_SAFARI: boolean; declare const IS_IOS: boolean; declare const IS_ANDROID: boolean; declare const IS_CHROME: boolean; declare const IS_ANDROID_CHROME: boolean; declare const IS_APPLE_WEBKIT: boolean; /** * Copyright (c) Meta Platforms, Inc. and affiliates. * * This source code is licensed under the MIT license found in the * LICENSE file in the root directory of this source tree. * */ type DFSNode = Readonly<{ depth: number; node: LexicalNode; }>; /** * Takes an HTML element and adds the classNames passed within an array, * ignoring any non-string types. A space can be used to add multiple classes * eg. addClassNamesToElement(element, ['element-inner active', true, null]) * will add both 'element-inner' and 'active' as classes to that element. * @param element - The element in which the classes are added * @param classNames - An array defining the class names to add to the element */ declare function addClassNamesToElement(element: HTMLElement, ...classNames: Array): void; /** * Takes an HTML element and removes the classNames passed within an array, * ignoring any non-string types. A space can be used to remove multiple classes * eg. removeClassNamesFromElement(element, ['active small', true, null]) * will remove both the 'active' and 'small' classes from that element. * @param element - The element in which the classes are removed * @param classNames - An array defining the class names to remove from the element */ declare function removeClassNamesFromElement(element: HTMLElement, ...classNames: Array): void; /** * Returns true if the file type matches the types passed within the acceptableMimeTypes array, false otherwise. * The types passed must be strings and are CASE-SENSITIVE. * eg. if file is of type 'text' and acceptableMimeTypes = ['TEXT', 'IMAGE'] the function will return false. * @param file - The file you want to type check. * @param acceptableMimeTypes - An array of strings of types which the file is checked against. * @returns true if the file is an acceptable mime type, false otherwise. */ declare function isMimeType(file: File, acceptableMimeTypes: Array): boolean; /** * Lexical File Reader with: * 1. MIME type support * 2. batched results (HistoryPlugin compatibility) * 3. Order aware (respects the order when multiple Files are passed) * * const filesResult = await mediaFileReader(files, ['image/']); * filesResult.forEach(file => editor.dispatchCommand('INSERT_IMAGE', { * src: file.result, * })); */ declare function mediaFileReader(files: Array, acceptableMimeTypes: Array): Promise>; /** * "Depth-First Search" starts at the root/top node of a tree and goes as far as it can down a branch end * before backtracking and finding a new path. Consider solving a maze by hugging either wall, moving down a * branch until you hit a dead-end (leaf) and backtracking to find the nearest branching path and repeat. * It will then return all the nodes found in the search in an array of objects. * @param startingNode - The node to start the search, if ommitted, it will start at the root node. * @param endingNode - The node to end the search, if ommitted, it will find all descendants of the startingNode. * @returns An array of objects of all the nodes found by the search, including their depth into the tree. * {depth: number, node: LexicalNode} It will always return at least 1 node (the ending node) so long as it exists */ declare function $dfs(startingNode?: LexicalNode, endingNode?: LexicalNode): Array; /** * Takes a node and traverses up its ancestors (toward the root node) * in order to find a specific type of node. * @param node - the node to begin searching. * @param klass - an instance of the type of node to look for. * @returns the node of type klass that was passed, or null if none exist. */ declare function $getNearestNodeOfType(node: LexicalNode, klass: Klass): T | null; /** * Returns the element node of the nearest ancestor, otherwise throws an error. * @param startNode - The starting node of the search * @returns The ancestor node found */ declare function $getNearestBlockElementAncestorOrThrow(startNode: LexicalNode): ElementNode; type DOMNodeToLexicalConversion = (element: Node) => LexicalNode; type DOMNodeToLexicalConversionMap = Record; /** * Starts with a node and moves up the tree (toward the root node) to find a matching node based on * the search parameters of the findFn. (Consider JavaScripts' .find() function where a testing function must be * passed as an argument. eg. if( (node) => node.__type === 'div') ) return true; otherwise return false * @param startingNode - The node where the search starts. * @param findFn - A testing function that returns true if the current node satisfies the testing parameters. * @returns A parent node that matches the findFn parameters, or null if one wasn't found. */ declare const $findMatchingParent: { (startingNode: LexicalNode, findFn: (node: LexicalNode) => node is T): T | null; (startingNode: LexicalNode, findFn: (node: LexicalNode) => boolean): LexicalNode | null; }; /** * Attempts to resolve nested element nodes of the same type into a single node of that type. * It is generally used for marks/commenting * @param editor - The lexical editor * @param targetNode - The target for the nested element to be extracted from. * @param cloneNode - See {@link $createMarkNode} * @param handleOverlap - Handles any overlap between the node to extract and the targetNode * @returns The lexical editor */ declare function registerNestedElementResolver(editor: LexicalEditor, targetNode: Klass, cloneNode: (from: N) => N, handleOverlap: (from: N, to: N) => void): () => void; /** * Clones the editor and marks it as dirty to be reconciled. If there was a selection, * it would be set back to its previous state, or null otherwise. * @param editor - The lexical editor * @param editorState - The editor's state */ declare function $restoreEditorState(editor: LexicalEditor, editorState: EditorState): void; /** * If the selected insertion area is the root/shadow root node (see {@link lexical!$isRootOrShadowRoot}), * the node will be appended there, otherwise, it will be inserted before the insertion area. * If there is no selection where the node is to be inserted, it will be appended after any current nodes * within the tree, as a child of the root node. A paragraph node will then be added after the inserted node and selected. * @param node - The node to be inserted * @returns The node after its insertion */ declare function $insertNodeToNearestRoot(node: T): T; /** * Wraps the node into another node created from a createElementNode function, eg. $createParagraphNode * @param node - Node to be wrapped. * @param createElementNode - Creates a new lexical element to wrap the to-be-wrapped node and returns it. * @returns A new lexical element with the previous node appended within (as a child, including its children). */ declare function $wrapNodeInElement(node: LexicalNode, createElementNode: () => ElementNode): ElementNode; type ObjectKlass = new (...args: any[]) => T; /** * @param object = The instance of the type * @param objectClass = The class of the type * @returns Whether the object is has the same Klass of the objectClass, ignoring the difference across window (e.g. different iframs) */ declare function objectKlassEquals(object: unknown, objectClass: ObjectKlass): boolean; /** * Filter the nodes * @param nodes Array of nodes that needs to be filtered * @param filterFn A filter function that returns node if the current node satisfies the condition otherwise null * @returns Array of filtered nodes */ declare function $filter(nodes: Array, filterFn: (node: LexicalNode) => null | T): Array; /** * Appends the node before the first child of the parent node * @param parent A parent node * @param node Node that needs to be appended */ declare function $insertFirst(parent: ElementNode, node: LexicalNode): void; /** * Calculates the zoom level of an element as a result of using * css zoom property. * @param element */ declare function calculateZoomLevel(element: Element | null): number; export { $dfs, $filter, $findMatchingParent, $getNearestBlockElementAncestorOrThrow, $getNearestNodeOfType, $insertFirst, $insertNodeToNearestRoot, $restoreEditorState, $wrapNodeInElement, CAN_USE_BEFORE_INPUT, CAN_USE_DOM, type DFSNode, type DOMNodeToLexicalConversion, type DOMNodeToLexicalConversionMap, IS_ANDROID, IS_ANDROID_CHROME, IS_APPLE, IS_APPLE_WEBKIT, IS_CHROME, IS_FIREFOX, IS_IOS, IS_SAFARI, addClassNamesToElement, calculateZoomLevel, isMimeType, markSelection, mediaFileReader, mergeRegister, objectKlassEquals, positionNodeOnRange, registerNestedElementResolver, removeClassNamesFromElement }; ```

[StyleT]

@etrepum I'll take a look later today/Monday. Thanks for the input!

[etrepum]

@StyleT here is a branch that uses tsup and fixes the problems I noticed, but I should do some more validation to make sure it's not missing anything else before bringing it in to #5876 https://github.com/etrepum/lexical/tree/tsup-build

[StyleT]

Short summary here:

• tsup doesn't work well because:
  • It repeats the copyright header a lot (once per source file that was rolled up)
  • It will bring in possibly unwanted global declarations, like this one (see above)
  • It fails to compile some packages as it aims esnext target w/o ability to override it https://github.com/egoist/tsup/blob/00188a0dc848c48fac45de245d1e021f370a84a3/src/rollup.ts#L147
• dts-bundle-generator is painfully slow and requires excessive configuration as it doesn't works same was as tsc
• api-extractor fails to figure out types for vars where we do destruction (known issue)
• rollup-plugin-ts requires rollup 3.x+ and has same issues as tsup except last one

So this solution seems optimal

[timocov]

dts-bundle-generator is painfully slow

@StyleT how can I reproduce this issue? Happy to take a look into and hopefully fix it.

requires excessive configuration as it doesn't works same was as tsc

Also would like to know more about this issue, if you have time.

Thanks!