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

Support custom extensions with allowImportingTsExtensions #621

Merged
merged 4 commits into from
Sep 25, 2023

Conversation

dfreeman
Copy link
Member

This is an alternative approach to the one taken in #620, and fixes #618.

#620 attempts to support import paths with an explicit .gts extension by rewriting those imports during transformation, but module resolution really shouldn't be a concern of the transform layer. For example:

  • It's important that we fix Support import paths with .gts extensions #618 in a way that works correctly even in files that we aren't transforming at all (consider that vanilla .ts files might import from .gts files)
  • Glint is already required to intercept TypeScript's view of the project files in ways that the transform layer can't impact (consider that we already support imports of .gts files without the extension)

TS has a hard-coded internal set of the extensions it considers to "be" TypeScript, and we work around this fact by presenting custom extensions like .gjs and .gts as .js and .ts files respectively. This is how such files get included during whole-project typechecking, and is also how extensionless imports of .gts and .gjs are resolved today.

This PR addresses #618 in the same manner as we handle custom extensions in general: by providing a custom hook for the compiler API to implement the behavior we want. In this instance, the hook is resolveModuleNameLiterals rather than the system-level readFile and friends, but the gist is the same: when a .gts file is imported, we intercept and rewrite the request to as though it had been for the corresponding .ts file.

`);
});

test.runIf(semver.gte(typescript.version, '5.0.0'))(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is a cool util! test.runIf!

Copy link
Contributor

@lukemelia lukemelia left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 Much better in every way than #620 !

@lukemelia
Copy link
Contributor

lukemelia commented Sep 25, 2023

Also, I tried this branch of glint against the project we are trying to convert to a v2 addon, which contains imports with .gts extensions, and it appears to build and typecheck successfully.

@dfreeman dfreeman merged commit 2c66bab into main Sep 25, 2023
4 checks passed
@dfreeman dfreeman deleted the explicit-gts-extensions branch September 25, 2023 16:25
@dfreeman dfreeman added the enhancement New feature or request label Sep 25, 2023
@dfreeman
Copy link
Member Author

Should be available in @glint/[email protected]!

@NullVoxPopuli
Copy link
Contributor

thanks! Already PR'd an update here: embroider-build/addon-blueprint#206

@bartocc
Copy link

bartocc commented Sep 26, 2023

Things are moving fast, it's awesome 👏

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

Successfully merging this pull request may close these issues.

Support import paths with .gts extensions
4 participants