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

Convert to V2 Addon #158

Draft
wants to merge 21 commits into
base: master
Choose a base branch
from

Conversation

NullVoxPopuli
Copy link
Collaborator

@NullVoxPopuli NullVoxPopuli commented Jun 14, 2022

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 🙃

How we got here (in order)

  1. Move ember-modifier to dependencies #127
  2. Remove @tailwindcss/ui #147
  3. Switch to pnpm #148
  4. Convert to monorepo #152
  5. Move tests to own app #157
  6. and now this PR! (Convert to V2 Addon #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

@NullVoxPopuli NullVoxPopuli added enhancement New feature or request breaking PR is considered a breaking change internal Internal changes that still require a mention in the changelog/release notes labels Jul 10, 2022
@rreckonerr
Copy link
Contributor

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

@tniezurawski
Copy link

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
Copy link
Collaborator Author

NullVoxPopuli commented Sep 28, 2023

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking PR is considered a breaking change enhancement New feature or request internal Internal changes that still require a mention in the changelog/release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants