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

import moment Error - only during embroider-safe scenario on Ember 4.12 addon (v1) #1598

Open
Pixelik opened this issue Sep 13, 2023 · 17 comments

Comments

@Pixelik
Copy link

Pixelik commented Sep 13, 2023

Hello,

I have the following setup in my ember-addon (v1) :

"devDependencies": {
  .....,
  ...,
  "@babel/core": "^7.22.1",
  "@babel/eslint-parser": "^7.21.8",
  "@babel/plugin-proposal-decorators": "^7.21.0",
  .....,
  ...,
  "@embroider/test-setup": "^3.0.0", // I have no other @embroider dependencies explicitly defined
  .....,
  ...,
  "ember-auto-import": "^2.6.3",
  .....,
  ...,
  "ember-source": "~4.12.0",
  .....,
  ...,
  "webpack": "^5.87.0",
  .....,
  ...
},
"resolutions" :{
  "@embroider/macros": "^1.12.2"
}

All of my automated tests (Acceptance, Integration, etc.) are passing in the default scenario

But the embroider-safe test scenario (ember-try) is throwing these 2 errors ❌ :

  1. qunit_parameterize__WEBPACK_IMPORTED_MODULE_7___default(...) is not a function
  2. Error: Could not find module moment imported from (require)

These errors are thrown due to these imports :

  • import moment from 'moment'
  • import { cases } from 'qunit-parameterize'

Note:

  1. @embroider/test-setup was updated from v2 to v3 by ember-cli-update --- to 4.12 and if I donwgrade it to v2 afterwards myself, the embroider-safe scenario throws TypeError: this.macrosConfig.packageMoved is not a function even before it starts executing tests
  2. I had to pin "@embroider/macros": "^1.12.2" in resolutions otherwise I get @babel errors when running ember serve even in the default scenario.
@Pixelik Pixelik changed the title embroider-safe throws webpack import error on Ember 4.12 addon (v1) embroider-safe Scenario throws webpack import error on Ember 4.12 addon (v1) Sep 13, 2023
@ef4
Copy link
Contributor

ef4 commented Sep 13, 2023

Looking at the published qunit-parameterize 0.4.0, it doesn't contain any default entrypoint. And the code it does contain doesn't export any name like cases.

Perhaps you added some custom configuration to autoImport to make that work? If so, you'll need to also add that kind of custom configuration to embroider.

@Pixelik
Copy link
Author

Pixelik commented Sep 13, 2023

I simply have ember-cli-qunit-parameterize v2 in devDependencies without any custom configuration and it's been working fine with ember-source": "~4.8.0

I'm guessing there is an issue here: https://github.com/BBVAEngineering/ember-cli-qunit-parameterize/blob/master/index.js#L8 when on Ember 4.12 🤔

Well I can always remove qunit-parameterize and replace it with Qunit.test.each but why am I having a similar issue with import moment from 'moment' ?

@Pixelik
Copy link
Author

Pixelik commented Sep 13, 2023

I stopped using qunit-parameterize and replaced it with Qunit.test.each

But I'm still having a similar error with moment:

Error: Could not find module moment imported from (require)

@Pixelik Pixelik closed this as completed Sep 13, 2023
@Pixelik Pixelik reopened this Sep 13, 2023
@Pixelik Pixelik changed the title embroider-safe Scenario throws webpack import error on Ember 4.12 addon (v1) moment import error only during embroider-safe scenario on Ember 4.12 addon (v1) Sep 13, 2023
@Pixelik Pixelik changed the title moment import error only during embroider-safe scenario on Ember 4.12 addon (v1) moment import Error only during embroider-safe scenario on Ember 4.12 addon (v1) Sep 13, 2023
@Pixelik
Copy link
Author

Pixelik commented Sep 14, 2023

If I understand correctly, same as with qunit-parameterize, the moment module itself does not have a default method, but combined with the ember-moment module, import moment from 'moment' then works.

So I believe the issue with Ember 4.12/Embroider-Safe is here somewhere?

@Pixelik
Copy link
Author

Pixelik commented Sep 14, 2023

Is it related to embroider-build/ember-auto-import#578 (comment) @ef4 ? 🤔

@void-mAlex
Copy link
Collaborator

@Pixelik do you have moment as a dev or peer dependency instead of a dev dependency?

@Pixelik
Copy link
Author

Pixelik commented Sep 14, 2023

I have moment under devDependencies and under peerDependencies

@void-mAlex
Copy link
Collaborator

does it work if you put it under dependencies instead?
(the way you have it, I would consider the correct way to have it configured)

@Pixelik
Copy link
Author

Pixelik commented Sep 14, 2023

It works if I add moment under dependencies and remove it from peerDependencies, even though it's not the correct way to do it 😕

So I guess I should close this issue? 🤷‍♂️

But if I don't declare it as a peerDependency there might be conflicts in the app that will be consuming my addon (the app might be installing its own version of moment which could be the wrong one)

@Pixelik Pixelik changed the title moment import Error only during embroider-safe scenario on Ember 4.12 addon (v1) import moment Error - only during embroider-safe scenario on Ember 4.12 addon (v1) Sep 14, 2023
@void-mAlex
Copy link
Collaborator

I suspect there is a bug somewhere with detecting which packages are allowed to be imported when a v1 addon has a peer dep declared, so I think this is a valid issue

@Pixelik
Copy link
Author

Pixelik commented Sep 19, 2023

Does anyone more knowledgable with embroider have any ideas about this ?
My current fix works but it is a temporary one and not a safe one 😞 🤔 🙏

@NullVoxPopuli
Copy link
Collaborator

what package manager and version are you using?

@Pixelik
Copy link
Author

Pixelik commented Sep 19, 2023

yarn 1.22.19

@void-mAlex
Copy link
Collaborator

@NullVoxPopuli I've seen this myself with pnpm 8.7.4 as well

@Pixelik
Copy link
Author

Pixelik commented Sep 20, 2023

the index.js file of my addon contains:

'use strict'

const validatePeerDependencies = require('validate-peer-dependencies')

module.exports = {
  name: require('./package').name,

  included(parent) {
    this._super.included.apply(this, arguments)

    validatePeerDependencies(__dirname, {
      resolvePeerDependenciesFrom: parent.root
    })
  }
}

is it possible that this is to blame when moment is defined under peerDependencies and running the embroider-safe scenario (embroider v3) on Ember 4.12 ?

@void-mAlex
Copy link
Collaborator

@Pixelik can you please confirm one more thing for me,
in your addon (if it's public would be good to get a link)
do you have ember-auto-import listed under dependencies ?

@Pixelik
Copy link
Author

Pixelik commented Sep 26, 2023

The repo is private I'm afraid and yes we have "ember-auto-import": "^2.6.3" under dependencies

"dependencies": {
    "ember-auto-import": "^2.6.3",
    "ember-cli-babel": "^7.26.11",
    "ember-cli-htmlbars": "^6.2.0",
    "ember-cli-showdown": "^6.0.0",
    ......,
    ....,
    "ember-feature-flags": "^6.0.0",
    "ember-modifier": "^3.2.7",
    "ember-truth-helpers": "^3.0.0",
    "moment": "^2.29.4",
    "validate-peer-dependencies": "^2.1.0"
  },
  "devDependencies": {
    "@arkweid/lefthook": "^0.7.2",
    "@babel/core": "^7.22.1",
    "@babel/eslint-parser": "^7.21.8",
    "@babel/plugin-proposal-decorators": "^7.21.0",
    "@commitlint/cli": "^13.1.0",
    "@ember-intl/cp-validations": "^5.0.0",
    "@ember/optional-features": "^2.0.0",
    "@ember/string": "^3.0.1",
    "@ember/test-helpers": "^2.9.3",
    "@embroider/test-setup": "^3.0.0",
    "@glimmer/component": "^1.1.2",
    "@glimmer/tracking": "^1.1.2",
    ........,
    ....,
    "broccoli-asset-rev": "^3.0.0",
    "concurrently": "^8.0.1",
    "ember-circleci": "^2.1.0",
    "ember-cli": "~4.12.2",
    "ember-cli-code-coverage": "^2.0.0",
    "ember-cli-dependency-checker": "^3.3.1",
    "ember-cli-flash": "^4.0.0",
    "ember-cli-inject-live-reload": "^2.1.0",
    "ember-cli-mirage": "^2.4.0",
    "ember-cli-sass": "^11.0.1",
    "ember-cli-sri": "^2.1.1",
    "ember-cli-terser": "^4.0.2",
    "ember-concurrency": "^3.0.0",
    "ember-data": "~4.12.3",
    "ember-exam": "^6.0.1",
    "ember-fetch": ">= 8.1",
    "ember-intl": "^6.0.0",
    "ember-load-initializers": "^2.1.2",
    "ember-moment": "^10.0.0",
    "ember-overridable-computed": "^1.0.0",
    "ember-page-title": "^7.0.0",
    "ember-qunit": "^6.2.0",
    "ember-resolver": "^10.0.0",
    "ember-source": "~4.12.0",
    "ember-source-channel-url": "^3.0.0",
    "ember-svg-jar": "^2.2.3",
    "ember-template-lint": "^5.7.2",
    "ember-test-selectors": "^6.0.0",
    "ember-try": "^2.0.0",
    "eslint": "^8.40.0",
    "eslint-config-prettier": "^8.8.0",
    "eslint-plugin-n": "^15.7.0",
    "eslint-plugin-prettier": "^4.2.1",
    "eslint-plugin-qunit": "^7.3.4",
    "loader.js": "^4.7.0",
    "moment": "^2.29.4",
    "prettier": "^2.8.7",
    "qunit": "^2.19.4",
    "qunit-dom": "^2.0.0",
    "sass": "^1.37.5",
    "stylelint": "^15.4.0",
    "stylelint-config-standard": "^32.0.0",
    "stylelint-prettier": "^3.0.0",
    "tracked-toolbox": "^2.0.0",
    "webpack": "^5.87.0"
  },
  "peerDependencies": {
    "ember-source": "^4.8.0"
  },
  "resolutions": {
    "@embroider/macros": "^1.12.2"
  },
  "engines": {
    "node": "16.* || >=18"
  },
  "ember": {
    "edition": "octane"
  },

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants