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

V2 Addon Conversion #1471

Merged
merged 10 commits into from
Aug 20, 2024
Merged

V2 Addon Conversion #1471

merged 10 commits into from
Aug 20, 2024

Conversation

NullVoxPopuli
Copy link
Sponsor Collaborator

@NullVoxPopuli NullVoxPopuli commented Jul 29, 2024

PRs to extract:


Steps:

  • cd addon
  • Upgrade local ember-cli
  • npx ember-cli@latest init --blueprint @embroider/addon-blueprint --typescript --addon-only
  • work through diffs
  • add file extensions to all imports (required by the default rollup config)
  • pnpm lint:fix
  • manually address / disable remaining lint failures

// @ts-ignore
import { precompileTemplate } from '@ember/template-compilation';

const OUTLET_TEMPLATE = precompileTemplate(`{{outlet}}`, { strictMode: false });
Copy link
Sponsor Collaborator Author

Choose a reason for hiding this comment

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

I didn't want to import from ember-cli-htmlbars, so I used @ember/template-compilation for precompileTemplate

Copy link
Sponsor Collaborator Author

Choose a reason for hiding this comment

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

This could also be pulled out to a separate PR, if folks want

// here; also see https://github.com/emberjs/data/issues/4071 for context
let setupContainer = require('ember-data/setup-container')['default'];
setupContainer(owner);
if (macroCondition(dependencySatisfies('ember-data', '>= 2.3'))) {
Copy link
Sponsor Collaborator Author

Choose a reason for hiding this comment

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

This change is also meaningful -- we don't want to use requirejs -- we're trying to get rid of it.

Insetad, our abstraction over runtime module loader is provided by embroider-macros.

Additionally, I saw that in this file that setup-container is deprecated without replacement for removal in v6.

So I added another if condition around this code so we can hopefully be a little fault tolerant when ember-data v6 comes around.

Copy link
Sponsor Collaborator Author

Choose a reason for hiding this comment

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

This could also be pulled out to a separate PR, if folks want

It doesn't make sense to run tests for type-tests with ember-try, since they're in a different workspace and not using the app

Remove Glint
Need multi-lockfile thanks to TS
Copy link
Member

@wagenet wagenet left a comment

Choose a reason for hiding this comment

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

This seems likely to be fine. Hard to review!

@NullVoxPopuli NullVoxPopuli merged commit 381c22c into master Aug 20, 2024
23 checks passed
@NullVoxPopuli NullVoxPopuli deleted the the-v2-addon-conversion branch August 20, 2024 15:33
@github-actions github-actions bot mentioned this pull request Aug 20, 2024
@NullVoxPopuli NullVoxPopuli mentioned this pull request Aug 20, 2024
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants