-
Notifications
You must be signed in to change notification settings - Fork 137
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 TypeScript without ember-cli-typescript #1236
Conversation
28c0445
to
f46fcae
Compare
I probably broke something 😅
|
5f7762b
to
6cee7fe
Compare
Add ts-app-template
6cee7fe
to
726c163
Compare
// For TS, we defer to ember-cli-babel, and the setting for | ||
// "enableTypescriptTransform" can be set with and without | ||
// ember-cli-typescript | ||
return ['.wasm', '.mjs', '.js', '.json', '.hbs', '.ts']; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This introduces a bug that causes TypeScript component files to be ignored when staticComponents: true
should be: return ['.ts', '.wasm', '.mjs', '.js', '.json', '.hbs'];
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
how does that happen / how does moving ts to the front resolve it? 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not at all familiar with the internals of embroider and had to diagnose this by trial-and-error.
The previous version of the code unshifts '.ts'
onto the array (placing it at the front). This change moves it to the end of the array.
When I inspected $TMPDIR/embroider/.../components, I found three files for each of my TypeScript components: .hbs, .js, and .ts.
.hbs and .ts were as expected. .js contains a templateOnlyComponent stub.
I can only deduce that in the process of assembling the app, embroider looks for the component implementation file in the order of the extensions in this array, so moving .ts to the end of the array means that it finds the empty stub implementation and ignores the actual implementation in the .ts file.
I have no idea why the stub .js implementations are being generated if a .ts backing class definition exists. Perhaps this is a different bug, or perhaps intended behavior?
The result is a broken component with no backing class.
Doesn't happen if staticComponents = false in the embroider config.
Repro'd and then patched here: NullVoxPopuli/ember-demo-typescript-without-ec-ts@dd5baa6
With glint +
<template>
, ember-cli-typescript's type-checking worker is incorrect as tsserver can't, by itself, resolve .gts imports.Since glint projects fully manage type checking within glint (and gts transformation via ember-template-imports), ember-cli-typescript should be removed as a requirement for projects using typescript so that we can reduce terminal noise in the build.
Later, we may bundle glint in some webpack (or preferrably, unplugin) plugin such that we properly type check ember projects with glint / template-imports (maybe similar to
fork-ts-checker--webpack-plugin
)anywho, this PR is mostly tests:
but also
Resolves: #889