-
-
Notifications
You must be signed in to change notification settings - Fork 260
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
V2 Addon Conversion #1471
Conversation
// @ts-ignore | ||
import { precompileTemplate } from '@ember/template-compilation'; | ||
|
||
const OUTLET_TEMPLATE = precompileTemplate(`{{outlet}}`, { strictMode: false }); |
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 didn't want to import from ember-cli-htmlbars, so I used @ember/template-compilation
for precompileTemplate
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 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'))) { |
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 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.
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 could also be pulled out to a separate PR, if folks want
6366b0d
to
fa96de8
Compare
6d7e763
to
3c062f6
Compare
3c062f6
to
d62c5cf
Compare
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
Fix docs command
d62c5cf
to
c25dacf
Compare
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 seems likely to be fine. Hard to review!
PRs to extract:
Drop support for TS < 5 #1474
require
Remove require usage, dropping support for automatic ember-data/setup-container inclusion #1472
Add Docs-Generation to CI -- will error if docs get out of sync and provides actionable information towards resolution. #1477
.npmrc
Use strict .npmrc #1475
pnpm-lock.yaml
(for more robust testing with both@types/*
and built-in types)Don't use a shared lockfile #1476
Steps:
cd addon
npx ember-cli@latest init --blueprint @embroider/addon-blueprint --typescript --addon-only
pnpm lint:fix