niklasf/chessops

Export everything from a single index.ts.

MicahZoltu opened this issue · 10 comments

MicahZoltu commented

import ... from '.../something' is not valid in ES6 modules as it mixes package import syntax and relative file import syntax. All exports should be consolidated in a single file that re-exports things from other files. You can either do namespaced re-exports or singular re-exports, depending on what makes the most sense for the project.

// apple.ts
export const apple = 'apple' as const

// banana.ts
export const peel = 'peel' as const

// index.ts
export { apple } from './apple'
export * as banana from './banana'
console.log(apple)
console.log(banana.peel)
ajrouvoet commented

This came up because I was trying to use chessops as a dependency and import one of the modules from it. For example:

import {Roles} from 'chessops/types' fails:

modules/.bin/tsc src/components/chess/Pieces.tsx
src/components/chess/Pieces.tsx:1:16 - error TS2724: Module '"../../../node_modules/chessops/types"' has no exported member 'Roles'. Did you mean 'ROLES'?

1 import {ROLES, Roles} from 'chessops/types'
                 ~~~~~

  node_modules/chessops/types.d.ts:12:22
    12 export declare const ROLES: Role[];
                            ~~~~~
    'ROLES' is declared here.


Found 1 error.
niklasf commented

That's suprising. Based on microsoft/TypeScript#8305 and https://www.typescriptlang.org/docs/handbook/module-resolution.html#node ff I expect this to work.

Example usage in lila, the main consumer of this library:

Maybe a subtle issue with moduleResolution or module type?

lila uses this to include only relevant modules instead of the whole bundle.

In any case I am not opposed to adding index.ts for convenience. Maybe keep the module structure as namespaces? Or would that be annoying to deal with on the consumer side? Would it be better flat?

ajrouvoet commented

Thanks for the quick response. Does lila install chessops as a dependency via npm or does it have a local checkout?

Edit: Ah I see already, installed as a dependency. Let me try and use lila's tsconfig to see if that changes anything.

ajrouvoet commented

Indeed, adopting the module(Resolution) parameters from lila seems to do the trick. That is fine for me. Perhaps others would still prefer a consolidated library export if they are stuck with or prefer other settings?

MicahZoltu commented

@niklasf Bundlers are, usually, able to handle package-relative imports and the output bundle is a single-file, so that is how most users "work around" (really, never notice) the problem. The problem crops up if you are trying to use native ES modules natively either in the browser or (I believe) in NodeJS 13+. In such situations, resolution fails because package-name/subdirectory isn't a known package (in browser packages must be mapped explicitly), but package-name is.

MicahZoltu commented

It is worth noting that I suspect the problem @ajrouvoet had may be unrelated, when they popped into the TypeScript discord seeking advice I noticed the package-relative import for their web project which is what lead me to file this issue. You are correct that TypeScript should be able to compile with package-relative imports, even if they won't work at runtime in a browser.

niklasf commented

Does 3d7e044 look reasonable? (Mostly flat re-exports, except for some non-essential modules.)

I'll keep the example/docs as is, because I believe the module-relative imports are still the best way to use this library in TypeScript or with bundlers.

ajrouvoet commented

Looks good to me, but I'm just starting out with typescript :)

niklasf commented

Published as v0.4.0.

MicahZoltu commented

Yeah, all looks reasonable to me, thanks!