search

embroider-build / embroider

Compiling Ember apps into spec-compliant, modern Javascript.
MIT License
330 stars 137 forks source link

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

Open Pixelik opened 8 months ago

Pixelik commented 8 months ago

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 :

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.
ef4 commented 8 months ago

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 commented 8 months ago

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 commented 8 months ago

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 commented 8 months ago

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 commented 8 months ago

Is it related to https://github.com/embroider-build/ember-auto-import/issues/578#issuecomment-1532096597 @ef4 ? 🤔

void-mAlex commented 8 months ago

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

Pixelik commented 8 months ago

I have moment under devDependencies and under peerDependencies

void-mAlex commented 8 months ago

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 commented 8 months ago

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)

void-mAlex commented 8 months ago

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 commented 8 months ago

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 commented 8 months ago

what package manager and version are you using?

Pixelik commented 8 months ago

yarn 1.22.19

void-mAlex commented 8 months ago

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

Pixelik commented 8 months ago

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 commented 8 months ago

@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 commented 8 months ago

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"
  },