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

Make explicit extensions in imports work. #659

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

NullVoxPopuli
Copy link
Contributor

@NullVoxPopuli NullVoxPopuli commented Dec 14, 2023

Resolves #628

See repro repo: https://stackblitz.com/edit/stackblitz-starters-g58nhp?file=examples%2Fts-addon-node16%2Fpackage.json,examples%2Fts-addon-node16%2Fsrc%2Findex.ts

  • pnpm build
  • poke around in the declarations directories

Using the patch from here: #628 (comment)
(where TS would allow)

And the test from here: https://github.com/typed-ember/glint/pull/648/files#diff-a835cb215198fc0a50973a4ae3bf66eac5a4b0bc21293fa29fc70888d663f797

The downside to this approach is "what if" someone has a foo.gts.ts file (probably co-located with a foo.gts.hbs file) -- but like, realistically, this isn't a thing they can use unless they're going half-in on static component references.

Notes:

Testing locally

The Glint Terminal

  • clone this branch
  • yarn
  • yarn build
  • delete glint-monorepo-test-utils from @glint/cores package.json's devDependencies

Repro Terminal

  • download extract and cd into the repro
  • pnpm i
  • pnpm link ~/path/to/this/branch/of/glint
  • pnpm build

note that after every pnpm link, in order to work in the glint repo again, you'll need to re-run yarn, and un-delete the glint-monorepo-test-utils package.

@NullVoxPopuli NullVoxPopuli marked this pull request as ready for review December 22, 2023 18:54
@lifeart
Copy link

lifeart commented Jan 23, 2024

Wondering, is allowArbitraryExtensions may help us here? https://www.typescriptlang.org/tsconfig#allowArbitraryExtensions
Also, looks like we may have customConditions https://www.typescriptlang.org/tsconfig#customConditions dynamically generated

Interesting part: {file basename}.d.{extension}.ts - is proposed type definition for custom extensions, so, instead of having component.gts.d.ts we may need to use component.d.gts.ts

image image https://github.com/microsoft/TypeScript/issues/50133

@NullVoxPopuli
Copy link
Contributor Author

@lifeart yeah, that sounds like a reasonable feature -- but we'd still need glint to to remove the extension during emitting.

So far, I've found it easiest, to work around the problem :(
Not ideal, but I don't know what the correct fix is yet. Maybe it's what you link to!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Import of .gts emits broken declarations
2 participants