Support ember-modifier v4 & ember-style-modifier v2 + v3

[mkszepp]

fix #689

Changes:

• Widen version range for ember-modifier to include ^4.0.0
• Add ember-auto-import@^2 as a dependency (breaking change)
• Drop support for node 12 (breaking change)

Cause of ember-modifier@4 we are already have braking, so can also support more versions of ember-style-modifier (v2 + v3). This versions are bringing also breaking changes.

• v2 needs ember-auto-import@^2
• v3 requires peerDependency @ember/string (this warning starts with ember v4.10)

[jelhan]

Tests are failing due to potential memory leak detected:

ember-cli-memory-leak-detector: Memory leak detected. 
The following classes were retained in the heap after tests completed: 
  BasicDropdownTrigger x2
  BasicDropdownContent x1
  BasicDropdown x3

I don't have much experience with ember-cli-memory-leak-detector. Maybe it is just a false positive? If not, it is likely a regression in an upstream dependency. I think we should first restart the test and see if it is passing in a second try.

[mkszepp]

@jelhan maybe caused by qunit > 2.14, see steveszc/ember-cli-memory-leak-detector#51

I have now recovered the old package.lock file & updated only the packages, which we want update

Update: Seen that we have already v2.19.3 installed, so maybe a other package updates brings this leaks...
Local with commitet package.lock i have no leaks, all tests were passed... I think in github test are alway using the latest version of packages... so we need to find out which packages brings this problems.

[mkszepp]

The ember-cli-memory-leak-detector v0.7.1 was the problem... now we use strict v0.5.1, which was installed since now. Starting with v0.6.1 there comes the error for memory leak

[jelhan]

@mkszepp Embroider optimized scenario is failing with the following error:

[Embroider:MacrosConfig] cannot read userConfigs until MacrosConfig has been finalized.

Could be caused by outdated Embroider version.

[jelhan]

I don't think you need having @ember/string as a peer dependency here. It should only be needed if the addon imports from @ember/string itself. Adding it to devDependencies should be enough if it only depends on another addon (here: ember-style-modifier), which requires that package. Adding it to devDependencies makes it available for the test app, which should be enough.

The difference is important if ember-style-modifier drops the peer dependency. If you have it declared as a peer dependency here as well, consumers would still need to install it even if it is not used anymore.

[mkszepp]

@jelhan sry my delay... thats right, i have removed the peerDependency

[jelhan]

@mkszepp @cibernox Please let me know if I can help getting this one in.

[cibernox]

Is expected that the embroider scenario is failing for these changes?

[mkszepp]

@cibernox no, the error Embroider optimized scenario, has nothing to do with this changes... i can also reproduce it with the current master version.

The error is maybe caused, because some packages are bringing as dependency ember-cli-htmlbars < 5.3.2. embroider-build/embroider#882

To fix the macro error, we need to update asome devDependencies...
I have tried this already some days ago, but the problem is, that some packages are very old, so we must remove old packages (if there is possible) or we create PR for this packages...

[jelhan]

@cibernox Current master seems to have similar problems as @mkszepp run into in this MR. E.g. latest scheduled run for master failed with the memory leak detection error, which @mkszepp fixed here. See https://github.com/cibernox/ember-basic-dropdown/actions/runs/4094738908/jobs/7061169669 for failing CI pipeline and #691 (comment) for information on the fix.

There seems to be many outdated dependencies making the tests unstable. I feel we need to get back to a stable state. In my experience upgrading one dependency at a time works out best. And using a tool like Dependabot or RenovateBot to reduce the manual workload. Main trade-off of this approach is that it requires many small PRs to be reviewed and merged over a couple of days or even weeks. @cibernox Do you have time doing so?

@mkszepp Maybe we could start by pulling out the fix for the memory leak issue in a separate PR and landing that one first? This also tells us if the fix for memory leak issue or the other upgrades in this PR caused the regression with Embroider tests.

[mkszepp]

@cibernox @jelhan The following packages are bringen an outdated ember-cli-htmlbar (we use v6.x)

• ember-code-snippet ^3.0.1"
• ember-cli-deprecation-workflow ^5.3.2 (already fixed, release is missing)
• [ember-cli-memory-leak-detector](https://github.com/steveszc/ember-cli-memory-leak-detector ^5.3.1 (we need to update to v0.7.1 so there is fixed
• ember-code-example-component ^5.3.2
• ember-concurrency `^5.7.1´
• ember-in-element-polyfill ^5.3.1
• ember-maybe-in-element ^5.2.0

Maybe only ember-code-snippet is causing our problem, because it is v3.x, all other are automatically latest ´v5.x` which would be ok.

I'm on the same opinion as @jelhan . We can merge this changes without any problems. After that, we should try to update ember-code-sinppet and making a PR, because i think after updating this package we can fix the Embroider optimized error.

When we have solved the Embroider optimized error we should also retry to update ember-cli-memory-leak-detector...

[mkszepp]

i have started a draft to update all dependencies manually (#696).

Now i will try to update ember-code-snippet` (lets see, in which error we run as next :) )

[mkszepp]

@jelhan @cibernox the macro problem occurs, when we use "ember-cli-htmlbars": "^6.2.0" as dependency. When we downgrade this dependency to ^5.3.2 the embroider tests are working. It looks like a mix of v5 & v6 are causing this problem... The update to v6.x was done in #631 (Okt 2021).

I have tested only the embroider try szenario's locally with this commit changes 9d6da21#diff-7ae45ad102eab3b6d7e7896acd08c427a9b25b346470d7bc6507b6481575d519 and it was fixed.

Using "ember-cli-htmlbars": "^5.3.2 || ^6.2.0" was also not helpful (build fails)