search

GavinJoyce / ember-headlessui

https://gavinjoyce.github.io/ember-headlessui/
Other
92 stars 34 forks source link

Convert to V2 Addon #158

Open NullVoxPopuli opened 2 years ago

NullVoxPopuli commented 2 years ago

We already require ember-auto-import@v2 so when this PR is complete, it would have been a non-breaking change -- however, I also dropped support / testing for ember-source < 3.28 for some sanity reasons. Previously 3.25 was tested against, but it wasn't until 3.27 that ember-source started shipping regular ESM. The differences are minor, but can surface issues in weird ways -- I mostly don't want to deal old code so.. here we have 3.28 as the minimum version now :upside_down_face:

How we got here (in order)

  1. https://github.com/GavinJoyce/ember-headlessui/pull/127
  2. https://github.com/GavinJoyce/ember-headlessui/pull/147
  3. https://github.com/GavinJoyce/ember-headlessui/pull/148
  4. https://github.com/GavinJoyce/ember-headlessui/pull/152
  5. https://github.com/GavinJoyce/ember-headlessui/pull/157

  6. and now this PR! (#158)

    How do you review something like this? (It's still a big PR) Points of interest:

    • the ci workflow is now generated from the root ci.yml, using: https://github.com/NullVoxPopuli/ember-ci-update
    • the rollup.config presently makes all files publicEntrypoints. We'll likely want to shift to gjs/gts and private component/modifier imports to reduce those publicEntrypoints -- because there is def stuff we don't want to be public API as most of this addon is to support the publicEntrypoints

    Things to keep in mind

    • v2 addons + monorepos simulate real npm package + app consumption as close as possible
    • we don't have any sort of deploy preview set up or anything, so you may want to see if the docs app boots up and looks fine
    • if all the status checks are green, we should be good to go
rreckonerr commented 1 year ago

@NullVoxPopuli Hi! Thanks for a great addon! Do you have any plans to make a new release of the addon any time soon?

tniezurawski commented 1 year ago

Great stuff @NullVoxPopuli! (as always). Do you remember what was wrong with this PR? The "CI / Build tests" job was failing but the artifacts are no longer available.

What is the work needed to push it through?

NullVoxPopuli commented 1 year ago

Yeah, i was having an issue with tests using apis that don't exist in the browser that were polyfilled by webpack4