adopted-ember-addons / ember-moment

MIT License
399 stars 122 forks source link

Import `bool` from @ember/object/computed #341

Closed aoumiri closed 3 years ago

aoumiri commented 3 years ago

This resolves https://github.com/stefanpenner/ember-moment/issues/340

There were 6 tests failing before making the change, not sure if that's something we should worry about?

aoumiri commented 3 years ago

Hello @stefanpenner, any idea on how to make the pipeline pass? It looks like the node version it's using is different from the one expected.

jacobq commented 3 years ago

It looks like the node version it's using is different from the one expected.

error resolve-package-path@3.1.0: The engine "node" is incompatible with this module. Expected version "10.* || >= 12". Got "8.17.0"
error Found incompatible module.
info Visit https://yarnpkg.com/en/docs/cli/install for documentation about this command.
The command "yarn install --no-lockfile --non-interactive" failed and exited with 1 during .

resolve-package-path v4.0.0 was just released (5/10/2021) also, which drops support for node 10. Seems like an appropriate time to drop node 8 and 10 here (both EOL). I'll take a shot at fixing CI in a separate PR and hopefully that'll help land this one (which is keeping me from being able to upgrade my app to 3.27).

resolve-package-path comes from ember-macro-helpers ``` $ yarn why resolve-package-path yarn why v1.22.5 [1/4] Why do we have the module "resolve-package-path"...? [2/4] Initialising dependency graph... [3/4] Finding dependency... [4/4] Calculating file sizes... => Found "resolve-package-path@1.2.7" info Reasons this module exists - "ember-macro-helpers#ember-cli-babel#broccoli-babel-transpiler#hash-for-dep" depends on it - Hoisted from "ember-macro-helpers#ember-cli-babel#broccoli-babel-transpiler#hash-for-dep#resolve-package-path" info Disk size without dependencies: "612KB" info Disk size with unique dependencies: "884KB" info Disk size with transitive dependencies: "928KB" info Number of shared dependencies: 3 Done in 0.79s. ```
jacobq commented 3 years ago

@aoumiri Try adding a yarn resolution to hold the offending package back like I did here (then rerun yarn): https://github.com/jacobq/ember-moment/commit/0b5456f3cac5dbc718ea69665da01935dd94b05c Alternatively, you can wait for the maintainers to drop Node 8 (and maybe also 10) support (requested in PR #346)

NullVoxPopuli commented 3 years ago

Thanks for the efforts!!! <3

This is now resolved (as computeds are not used at all in Octane+ ember-moment).

This'll be released in v9 shortly