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

Deprecate non-colocated components. #995

Merged
merged 15 commits into from
Mar 8, 2024
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
247 changes: 247 additions & 0 deletions text/0995-deprecate-non-colocated-components.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,247 @@
---
stage: accepted
start-date: 2023-12-15T00:00:00.000Z
release-date:
release-versions:
teams: # delete teams that aren't relevant
- cli
- framework
- learning
- typescript
prs:
accepted: https://github.com/emberjs/rfcs/pull/995
project-link:
---

<!---
Directions for above:

stage: Leave as is
start-date: Fill in with today's date, 2032-12-01T00:00:00.000Z
release-date: Leave as is
release-versions: Leave as is
teams: Include only the [team(s)](README.md#relevant-teams) for which this RFC applies
prs:
accepted: Fill this in with the URL for the Proposal RFC PR
project-link: Leave as is
-->

# Deprecate non-co-located components.

## Summary

Deprecates
- classic component layout
- pods component layout


Once this deprecation is implemented, only the following will be allowed:
- co-located components
- gjs / gts components

## Motivation

These older component layouts force build tooling to keep a lot of resolution rules around, and makes it hard for codemods and other community tooling to effectively work across folks' projects.


## Transition Path

There are two types of paths to migrate off the old layouts
- use a currently supported multi-file layout (keeping separate `js`, `ts`, and `hbs` files)
- migrate the component entirely to the latest component format, `gjs`, `gts`, (aka `<template>`)

There are some tools to help with this:
- [Classic to Colocation](https://github.com/ember-codemods/ember-component-template-colocation-migrator)
- [Convert pods to Colocation](https://github.com/ijlee2/ember-codemod-pod-to-octane)
- [Convert to `<template>`](https://github.com/IgnaceMaes/ember-codemod-template-tag)


Specifically, these layouts are no longer supported

### Classic

```
{app,addon}/
components/
foo.js
namespace/
bar.js
templates/
components/
foo.hbs
namespace/
bar.hbs
```

### Pods' Component layout.

```
{app,addon}/
components/
foo/
component.js
template.hbs
namespace/
bar/
component.js
template.hbs
```

The above example(s) could fairly easily be migrated (by users) to:

```
{app,addon}/
components/
foo.js
foo.hbs
namespace/
bar.js
bar.hbs
```

Or using `component-structure=nested`

```
{app,addon}/
components/
foo/
index.js
index.hbs
namespace/
bar/
index.js
index.hbs
```

Note, however, that classic components _importing_ the `layout` and setting it on an `@ember/component` will still work.
The key thing being deprecated is the runtime resolution of templates, so if there is an import involved, there is no runtime resolution.

### `ember-source`

The blueprint for components will need to remove the `classic` option:

Here is the output at the time of authoring this RFC:
```bash
❯ pnpm ember g component --help
Requested ember-cli commands:

ember generate <blueprint> <options...>
Generates new code from blueprints.
aliases: g
--dry-run (Boolean) (Default: false)
aliases: -d
--verbose (Boolean) (Default: false)
aliases: -v
--pod (Boolean) (Default: false)
aliases: -p, -pods
--classic (Boolean) (Default: false)
aliases: -c
--dummy (Boolean) (Default: false)
aliases: -dum, -id
--in-repo-addon (String) (Default: null)
aliases: --in-repo <value>, -ir <value>
--lint-fix (Boolean) (Default: true)
--in (String) (Default: null) Runs a blueprint against an in repo addon. A path is expected, relative to the root of the project.
--typescript (Boolean) Generates a version of the blueprint written in TypeScript (if available).
aliases: -ts

component <name> <options...>
Generates a component.
--path (String) (Default: components)
aliases: --no-path (--path="")
--component-class (@ember/component, @glimmer/component, @ember/component/template-only, "") (Default: --no-component-class)
aliases: -cc (--component-class=@ember/component), -gc (--component-class=@glimmer/component), -tc (--component-class=@ember/component/template-only), -nc (--component-class=""), --no-component-class (--component-class=""), --with-component-class (--component-class=@glimmer/component)
--component-structure (flat, nested, classic) (Default: flat)
aliases: -fs (--component-structure=flat), -ns (--component-structure=nested), -cs (--component-structure=classic)

```

Specifically, we'll remove:
- `--component-structure=classic` and its alias


## How We Teach This

Guides and docs already don't mention the above old layouts.

### Deprecation Guide

Copy link
Contributor Author

Choose a reason for hiding this comment

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

add some prose, links, explanation, etc

#### component templates in `templates/components`

**Before**
```
{app,addon}
components/
demo.js
templates/
components/
demo.hbs
```

**After**
```
{app,addon}
components/
demo.js
demo.hbs
```

**After (gjs)**
```
{app,addon}
components/
demo.gjs
```

#### Pods


**Before**
```
{app,addon}
components/
demo/
component.js
template.hbs
```

**After**
Copy link
Member

Choose a reason for hiding this comment

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

I feel like we should not be recommending the "index" variant for people because of a few bugs around that right now. Sure we can make it work but I think we can just suggest the "non-nested" as the after version in the RFC 🤷

Copy link
Contributor Author

@NullVoxPopuli NullVoxPopuli Mar 6, 2024

Choose a reason for hiding this comment

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

index is the transition path from component.js/template.hbs since Octane. It's a primary, first-class, feature -- following classic pre-ESM node resolution rules (where index is always the implicit target of a directory path). We can't just get rid of it (by omission) because there is a bug in the unstable embroider resolver (tho, I think that was fixed! (you fixed it! 💪 ))

Copy link
Contributor

Choose a reason for hiding this comment

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

We discussed in RFC review and agreed that showing index.js is OK because it's a supported node-ism that we need to continue to support, and any bugs are just bugs that we need to fix (in embroider).

```
{app,addon}
components/
demo/
index.js
index.hbs
```

**After (non-nested)**
```
{app,addon}
components/
demo.js
demo.hbs
```

**After (gjs)**
```
{app,addon}
components/
demo.gjs
```


## Drawbacks

If upgrading to ember v6 without any changes,
- Some super old addons may break
- Some super old apps may break

In either case, it's been ~4 years since co-located components were introduced, and the deprecation will give folks actionable information for how to move forward (ideally with a link to the deprecation site which can then link to tools folks can try).

## Alternatives

none

## Unresolved questions

none
Loading