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

Switch to maintenance mode #1568

Closed
NullVoxPopuli opened this issue Sep 7, 2023 · 18 comments · Fixed by #1576
Closed

Switch to maintenance mode #1568

NullVoxPopuli opened this issue Sep 7, 2023 · 18 comments · Fixed by #1576

Comments

@NullVoxPopuli
Copy link
Collaborator

NullVoxPopuli commented Sep 7, 2023

Blockers:

@basz
Copy link

basz commented Sep 11, 2023

ember-cli-typescript can be removed when using ember-cli-babel v8?

@NullVoxPopuli
Copy link
Collaborator Author

it can be removed when using ember-cli-babel v7, provided you add the config in ember-cli-build.js

@LucasHillDex
Copy link

@NullVoxPopuli What version of ember is required to get built in types and TS generators?

@NullVoxPopuli
Copy link
Collaborator Author

NullVoxPopuli commented Sep 11, 2023

starting in 4.9, you can:

import 'ember-source/types';
import 'ember-source/types/preview';

However, ember-data is not compatible with these types, which is why they are currently opt-in.

Starting in 5.1, you can only have:

import 'ember-source/types';

For generators, you want ember-source 5.1+, iirc

@basz
Copy link

basz commented Sep 11, 2023

An in-repo addon does not like that. Does that need something special?

@NullVoxPopuli
Copy link
Collaborator Author

For addons, you'd add to the index.js:

// <v1-addon-name>/index.js
module.exports = {
  name: require('package').name,
  options: {
    'ember-cli-babel': {
      enableTypeScriptTransform: true
    }
  }
}

@abeforgit
Copy link

and just to confirm: the prepack and postpack scripts that ran ember ts:precompile and ember ts:clean are just no longer needed at all?

abeforgit added a commit to lblod/ember-rdfa-editor-lblod-plugins that referenced this issue Sep 11, 2023
as described in [this pr](typed-ember/ember-cli-typescript#1568)
ember-cli-typescript is no longer needed, as are the prepack and
postpack scripts.

On top of removing an unnecessary dep, this actually solved an issue
allowing us to use yalc, which is very useful for "proper" npm linking
that simulates an actual package install (which can really help in
certain peerdep scenarios)
abeforgit added a commit to lblod/ember-rdfa-editor-lblod-plugins that referenced this issue Sep 11, 2023
as described in [this pr](typed-ember/ember-cli-typescript#1568)
ember-cli-typescript is no longer needed, as are the prepack and
postpack scripts.

On top of removing an unnecessary dep, this actually solved an issue
allowing us to use yalc, which is very useful for "proper" npm linking
that simulates an actual package install (which can really help in
certain peerdep scenarios)
@NullVoxPopuli
Copy link
Collaborator Author

NullVoxPopuli commented Sep 11, 2023

they are aliases to commands that you could run yourself, yea.
the ts:precompile one is involved, and could maybe be simplified.

With v2 addons, the ts:* commands aren't needed at all.
(declarations for v2 addons are built in to a directory (we chose declarations), and then package.json#exports tells the tooling where to look for both modules and types: https://github.com/embroider-build/addon-blueprint/blob/main/files/__addonLocation__/package.json#L98

@abeforgit
Copy link

ah but for v1 addons, and for integration with typescript hosts, running the underlying commands would still be required to allow type imports, I gather?

@NullVoxPopuli
Copy link
Collaborator Author

correct -- a similar thing occurs for the v2-addon blueprint, but all the CLI flags are abstracted to the tsconfig.json

@abeforgit
Copy link

In that context though, for v1 addons, the two commands have definite value since they're non-trivial to recreate (theres quite some file copying happening that I don't really want to redo for every project)

so this addon could reduce scope to those two commands rather than being archived outright?
or better to move the commands out of here and maintain as a separate project?

@NullVoxPopuli
Copy link
Collaborator Author

I can't speak for the typed-ember team, but I think it totally fine to install and still use an archived project.

v2 addons will be the default in Polaris though, and there are loads of performance benefits to v2 addons - so I would encourage everyone migrate to v2 addons at some point (ie: not yet, still some bugs to work out (unless you have an appetite for combing through non-blueprint v2 addons)).

But, most importantly, I don't foresee anything needing to change aside from the above mentioned list in description of this issue (thus, if archived, no development would be needed, but it doesn't mean its taken down from npm -- folks can still use the package).

@basz
Copy link

basz commented Sep 18, 2023

I have an app with many in repo addons. After doing removing ember-cli-typescript from all package.json and adding the above js hints my app seems to work fine. However I still see WARNING: ember-cli-typescript requires ember-cli-babel ^7.17.0, but you have version 8.0.0 installed; your TypeScript files may not be transpiled correctly.. I'm assuming that comes from one of the many external addons I have installed. Just wanted to make sure that is not an issue?

abeforgit added a commit to lblod/ember-rdfa-editor-lblod-plugins that referenced this issue Sep 18, 2023
* fix(deps): bump ember deps and add them to peerdeps

* docs(changelog): add changeset

* fix(typescript): remove ember-cli-typescript

as described in [this pr](typed-ember/ember-cli-typescript#1568)
ember-cli-typescript is no longer needed, as are the prepack and
postpack scripts.

On top of removing an unnecessary dep, this actually solved an issue
allowing us to use yalc, which is very useful for "proper" npm linking
that simulates an actual package install (which can really help in
certain peerdep scenarios)

* fix(types): add back ember cli, but as a devDep

Adding it as a dep is no longer needed since the compilation is handled
by babel.
Keeping it as a dep anyway somehow breaks the use of yalc (unknown why
this is)
Removing the dep altogether gets rid of the very useful precompile and
clean commands that are not easy to replicate.
So moving it to devDeps gives us the best of both worlds: working yalc,
working compilation, and working prepack and postpack scripts
@chriskrycho
Copy link
Member

@NullVoxPopuli we shouldn't archive this; we should put it into “maintenance mode”, with a clear indication that the only reason to use it is for v1 add-on which has not yet moved to v2. The reason not to archive the repo: if you archive it you lose the ability to do things like, say, apply critical security fixes to it. 😉 We don’t expect any of those, to be sure, but it’s much better to keep the repo available and keep its dependencies broadly up to date, while making it clear that there is no other reason to use it and recommending folks upgrade to the other paths.

A good example here is: we should absolutely fix the issue with ember-cli-babel and publish a small release that just unblocks that, so that people who are still using this do not see CI get blocked!

@NullVoxPopuli
Copy link
Collaborator Author

Does it then make sense to flag as deprecated on npm?

The thing i want to communicate is that ec-typescript isn't the future

@chriskrycho
Copy link
Member

No, absolutely not. It's not deprecated! It's just in maintenance mode. 😂 Clear documentation in the README and the docs should do the trick just fine! (And we'll merge PRs to that effect very quickly!)

@chriskrycho chriskrycho changed the title Archive this repo Switch to maintenance mode Sep 28, 2023
@abeforgit
Copy link

abeforgit commented Oct 20, 2023

I for one would like to see a recommendation of how to replace ember ts:precompile for V1 addon authors added to the todo list as well

@chriskrycho
Copy link
Member

That's a great callout. It's pretty easy to do now since we have the relevant "hack" for it with typesVersions.

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 a pull request may close this issue.

5 participants