ember-cli-typescript icon indicating copy to clipboard operation
ember-cli-typescript copied to clipboard

Docs: add info about merged app directory

Open chriskrycho opened this issue 6 years ago • 17 comments

We currently document one gotcha for addon authors building addons in TypeScript:

Because addons have no control over how files in app/ are transpiled, you cannot have .ts files in your addon's app/ folder.

To expand on that for future readers who may stumble on this (and as a starting point for the docs we should write up):

Having .js supplied to a specific path inside the app namespace by both an addon and the app itself, when one of them is using TypeScript to generate that file, creates a caching problem. When addons inject .js files into the app namespace, and we're compiling .ts files to .js files in the consuming app (always in the app namespace), or vice versa, we can end up in a situation where Ember CLI sees that there is already a .js file there, so it ignores the one generated from .ts. Which one it picks up seems to be semi-random (at least: we haven't chased it down to see if it's consistently replicable).

The real solution here is for addons to stop injecting themselves into the app namespace and to supply imports people can use instead to explicitly opt into using their functionality. However, until that becomes a community norm (best guess: not until Embroider lands), we should document this as a gotcha, including the kinds of build errors it produces, so that people find it when it happens to them and we have a useful place to direct addon authors to in understanding why and how they're breaking their consumers.

Ecosystem issues

  • #724
  • ember-cli/ember-ajax#428
  • stefanpenner/broccoli-persistent-filter#166

chriskrycho avatar Jul 31 '19 14:07 chriskrycho

// cc

I think it should be redlined and pinned in the docs - because this is floating behaviour and can create serious damage for business.

for case - we have application.ts route and have ember-simple-auth installed - it's so easy to get issue with inconsistent builds, because there is no user action needed for it. (and alot of ember users use ember-simple-auth)

lifeart avatar Jul 31 '19 14:07 lifeart

I've also gotten bitten by the ESA Application route class export stomping my TS class.

jordpo avatar Jan 06 '20 20:01 jordpo

I've also experienced the same issue with ember-simple-auth stomping my TS application route class.

LuisAverhoff avatar Jan 06 '20 20:01 LuisAverhoff

This is a big trap. Any app that consumes an addon that exports a file that matches the name of a file in the app will sporadically get a bad build.

ESA is one of the most popular add-ons, and it happens to export the application route. So that is why people have come across this issue using it.

Another very common case will be apps that consume app-specific addons (ones written just for one or two apps, not public consumption). The reason is this type of addon is often used to define a base which the one or more apps can extend. It is natural in this case for the files to have the same name in app and addon, and for there to be lots of them.

We were bitten by both the above scenarios (three apps use esa and consume two local add-ons). It was difficult to track down and cost us a bunch of time.

patrickberkeley avatar Jan 06 '20 21:01 patrickberkeley

Ran into this again. This time it's the Intl service from ember-intl: we customize it, but the original version gets loaded.

This happens only to me and does not happen to my teammates and CI. Extremely frustrating!

I strongly believe that, if ember-cli-typescript can't do anything about it, this must be recognized as an issue upstream and fixed there. Has the issue already been registered in the ember-cli repo or somewhere like that?

lolmaus avatar Jan 20 '20 12:01 lolmaus

Note that I can't use a different service name like app/services/intl-custom.ts. This would resolve the collision, but ember-intl tools, such as the t template helper, will still be using the original service!

Are you expecting me to fork ember-intl or what? I can't see a way out of this.

lolmaus avatar Jan 20 '20 12:01 lolmaus

OK, this seems to be the one: https://github.com/broccolijs/broccoli-persistent-filter/issues/166

lolmaus avatar Jan 20 '20 13:01 lolmaus

@lolmaus we have hacked around this by creating a -private directory for our TS files, then re-exporting from the a JS file at the normal position in the tree. Example for your situation:

// app/services/-private/intl.ts
// whatever code you need to customize…

Then re-export it in a JS file:

// app/services/intl.js
export { default } from `<your-app-or-addon-name>/services/-private/intl`;

This way you don't have two TS files with the same name in your tree.

patrickberkeley avatar Jan 20 '20 15:01 patrickberkeley

Happily we have a way to at least give you a warning here (see the PR @dfreeman just opened earlier today). The issue here is unfortunately entirely upstream from us and there's nothing we can do but warn.

The medium-term solution here is for app re-exports for addons to stop being a thing, and to use normal imports and resolution instead.

chriskrycho avatar Jan 20 '20 22:01 chriskrycho

Maybe it's possible to manually remove JS files from the build via an in-repo addon?

lolmaus avatar Jan 21 '20 06:01 lolmaus

Doing so would be inherently fragile and likely to break things, because people (for good or for ill) depend on that behavior across the ecosystem. You might be able to build something that works for your specific app, but unfortunately it wouldn't be generalizable. If you do, it'd be a great thing to blog about and link to from here for the sake of others hitting this pain point!

chriskrycho avatar Jan 21 '20 14:01 chriskrycho

Well, I'm basically asking if using broccoli-funnel in treeForApp is too late to remove the offending file from the build.

lolmaus avatar Jan 21 '20 15:01 lolmaus

This is a big trap. Any app that consumes an addon that exports a file that matches the name of a file in the app will sporadically get a bad build.

ESA is one of the most popular add-ons, and it happens to export the application route. So that is why people have come across this issue using it.

Another very common case will be apps that consume app-specific addons (ones written just for one or two apps, not public consumption). The reason is this type of addon is often used to define a base which the one or more apps can extend. It is natural in this case for the files to have the same name in app and addon, and for there to be lots of them.

We were bitten by both the above scenarios (three apps use esa and consume two local add-ons). It was difficult to track down and cost us a bunch of time.

so how can we solve this issue issue for ESA?

ahemed-haneen avatar Mar 19 '20 10:03 ahemed-haneen

There are multiple suggested workarounds in this thread. Those are the best options available at present, unfortunately.

chriskrycho avatar Mar 19 '20 12:03 chriskrycho

@TheLooseCannon this is our current approach: https://github.com/typed-ember/ember-cli-typescript/issues/780#issuecomment-576318921

patrickberkeley avatar Mar 19 '20 13:03 patrickberkeley

We ran into a similar issue with ember-redux. Our reducers/index.ts file needed to be renamed to index.js and the warning went away.

JonRossway avatar Jul 30 '20 21:07 JonRossway

We ran into this problem with app/services/store.{js,ts}. I think Ember Data provides a store.js in the app tree, if you care to override it it won't be used, otherwise the default implementation does what you want (re-export the ember data store as a service).

We did have a custom store subclass and hit this issue once we renamed it to .ts. The solution we went with (similar to what others have done) is to put our custom store in app/services/-store.ts, then add:

// app/services/store.js

export { default } from './-store';
export * from './-store';
// app/services/store.d.ts

export { default } from './-store';
export * from './-store';

I haven't tested that the .d.ts is actually necessary, or if TS will just follow the re-export from the js file and find the types. Either way, it seems to work well for us. It's unfortunate that it is necessary, but doesn't seem to have much other unwanted consequences.

chancancode avatar Sep 02 '20 22:09 chancancode

Have there been any updates as to the best practices to handle this, or perhaps modern / upcoming Ember APIs that might sidestep these issues?

machty avatar Dec 19 '22 08:12 machty

The workarounds described here remain (and will remain) the best options as long as Ember’s app-level build is still a “classic” build system reliant on the resolution and override rules it has used for roughly a decade now. When it is replaced with a v2 app authoring story (presumably using Vite or similar), the issue will be fundamentally resolved. In the meantime, this is inactionable on our end. Equally importantly, we now recommend that people switch to using ember-cli-babel for apps (as documented here) and the rollup plugin configured as part of the v2 add-on build for add-ons, in conjunction with running glint or tsc directly on their projects, so we are not further investing in fixing things like this in ember-cli-typescript. Closing accordingly!

chriskrycho avatar Sep 28 '23 22:09 chriskrycho