-
Notifications
You must be signed in to change notification settings - Fork 123
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
Convert to V2 Addon #2028
Convert to V2 Addon #2028
Conversation
ec01439
to
f99ab61
Compare
f99ab61
to
fff3f04
Compare
8910095
to
4b2aa65
Compare
…n-place Restore workflows, unchanged Put some documentation config / files back and get 'docs' building working Looks like all of ci.yml should work now Cleanup and get tests passing Add lint:fix script Get lints passing Move renovate config back out to the root release.yml should be up-to-date now as well Restore ember-try config Remove test:ember from the addon, because the test app has been extracted Update ci.yml for compatibility testing Move projects in to packages folder Move back out to root of directory, because now is not the time to restructure again -- can happen later
Remove old files and dependencies from qunit-dom Add new test index.html that demonstrates the problem in mainmatter#2022 Fix exports Ah, so we do compile to ESM already We have to remove the ember-addon keyword because we don't want to provide an addon-main and then have a dependency on @embroider/addon-shim
84e74e4
to
5ce0f55
Compare
@@ -38,34 +47,13 @@ | |||
"/dist/" | |||
] | |||
}, | |||
"dependencies": { |
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.
We need to include addon-shim here https://github.com/embroider-build/embroider/blob/main/packages/addon-shim/README.md
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.
we do not, because this isn't an ember addon, it's a vanilla package that non-ember projects can use.
Title is a bit of a lie, because this is now "just an npm package" no emberisms, allowing usage outside the ember ecosystem.
In technicality, "V2 Addons" are a migration process and spec for writing ember things as "just an npm package". Things like the addon-shim and other embroider-build/addon-blueprint (v2 addon blueprint) conventions are things that make traditional component-using addons easy for forwards and backwards compatibility. Since qunit-dom has no emberisms, we don't need to bring all of those compatibility / emberisms along with us.
Blocked on: #2027(The diff here will look huge until the above is merged).
Use this Diff
until the #2027 is merged.
NullVoxPopuli/qunit-dom@v2-addon...actual-v2-addon
It's a much smaller set of changes
Alternatively, this PR could be merged and we can ignore #2027
Actual changes here are 90+% removals.
However, I added a package that uses Vite to prove cross-compatibility with the wider ecosystem.
Without changing anything in qunit-dom, but just removing all the emberisms, we get an error
This PR resolves that.
Evidence:
Of note:
I had to remove the
ember-addon
keyword, because with it, we must provide an addon-main, and for V2 addons that would be depending on@embroider/addon-shim
, which I don't think we'd want to do.I was able to determine via the ember-try config that the min-supported ember-setup includes ember-qunit@v4, which precedes the ember-qunit where ember-auto-import@v2 is required: https://github.com/emberjs/ember-qunit/blob/main/addon/CHANGELOG.md#v600-2022-10-07 -- as a result, v2 conversion will be a breaking change, bumping qunit-dom to v3