salsify / ember-css-modules

CSS Modules for ambitious applications
MIT License
282 stars 50 forks source link

Building addon with 1.3.0-beta.1 fails with message `Unable to resolve styles from addon ; is it installed?` #166

Closed FabHof closed 4 years ago

FabHof commented 4 years ago

I updated my addon with pod-structure and ember-css-modules-sass to use ember-css-modules 1.3.0-beta.1, because I wanted to start to move from pod-structure to template co-location Now ember build fails with Unable to resolve styles from addon ; is it installed?

ember build output:

Environment: development
| BuildingBrowserslist: caniuse-lite is outdated. Please run next command `yarn upgrade caniuse-lite browserslist`
cleaning up...
Build Error (CSSModules)

Unable to resolve styles from addon ; is it installed?

Stack Trace and Error Report: C:\Users\hmf\AppData\Local\Temp/error.dump.bd6ac2ffb1f9850e1ea43f34fea293a1.log

Packages I have installed:


  "dependencies": {
    "@ember/render-modifiers": "^1.0.2",
    "@fortawesome/ember-fontawesome": "^0.1.14",
    "@fortawesome/free-solid-svg-icons": "^5.10.2",
    "ember-cli-babel": "^7.7.3",
    "ember-cli-htmlbars": "^3.0.1",
    "ember-cli-sass": "^10.0.1",
    "ember-css-modules": "^1.3.0-beta.1",
    "ember-css-modules-sass": "^1.0.1",
    "ember-decorators": "^6.1.1",
    "ember-intl": "^4.2.0",
    "ember-modal-dialog": "^3.0.0-beta.4",
    "ember-models-table": "^2.11.0",
    "ember-pikaday": "^2.4.0",
    "ember-power-select": "^2.3.5",
    "ember-progress-bar": "^1.0.0",
    "sass": "^1.22.10"
  },
  "devDependencies": {
    "@ember/optional-features": "^0.7.0",
    "babel-eslint": "^10.0.3",
    "broccoli-asset-rev": "^3.0.0",
    "ember-classic-decorator": "^1.0.5",
    "ember-cli": "~3.12.0",
    "ember-cli-dependency-checker": "^3.1.0",
    "ember-cli-eslint": "^5.1.0",
    "ember-cli-htmlbars-inline-precompile": "^2.1.0",
    "ember-cli-inject-live-reload": "^1.10.0",
    "ember-cli-moment-shim": "^3.7.1",
    "ember-cli-sri": "^2.1.1",
    "ember-cli-template-lint": "^1.0.0-beta.3",
    "ember-cli-uglify": "^2.1.0",
    "ember-disable-prototype-extensions": "^1.1.3",
    "ember-export-application-global": "^2.0.0",
    "ember-faker": "^1.5.0",
    "ember-load-initializers": "^2.0.0",
    "ember-maybe-import-regenerator": "^0.1.6",
    "ember-moment": "^7.8.1",
    "ember-qunit": "^4.4.1",
    "ember-resolver": "^5.0.1",
    "ember-source": "~3.12.0",
    "ember-source-channel-url": "^1.1.0",
    "ember-try": "^1.0.0",
    "eslint-plugin-ember": "^6.2.0",
    "eslint-plugin-node": "^9.0.1",
    "eslint-plugin-prettier": "^3.1.1",
    "husky": "^3.1.0",
    "loader.js": "^4.7.0",
    "prettier": "1.19.1",
    "pretty-quick": "^2.0.1",
    "qunit-dom": "^0.8.4"
  },
buschtoens commented 4 years ago

There's a "bug" in how the error message for missing / unresolvable @value ... from ... or composes imports is assembled. addonName is empty and the original importPath is not logged.

https://github.com/salsify/ember-css-modules/blob/bf70611e89e4e9b438b0ab76f496b2dc859f0153/packages/ember-css-modules/lib/resolve-path.js#L40-L47

You are trying to @value import an unresolvable file or virtual module. I think it's probably an unresolvable relative file path, that became invalid because you moved some files around.

Can you try adding console.log({ importPath }) between L45 and L46?

dfreeman commented 4 years ago

🤔 We shouldn't be entering resolveExternalPath at all for relative paths, but an empty addonName there is definitely a bug no matter what the input is. Thank you for helping pinpoint this @buschtoens!

@FabHof figuring out what importPath is triggering this will definitely help work out what's going wrong here. Hopefully if you can report back with that we'll be able to figure it out—thanks!

FabHof commented 4 years ago

Thanks for the responses, I did some digging and It seems to be an issue with a virtual module.

{
  importPath:"colors"
}

I have the following in my index.js:

  included(app) {
  // ...
    this.options = Object.assign({}, this.options, {
      cssModules: {
        virtualModules: {
          colors: {
            primary: config.primary || '#a55176'
            // some more colors
          }
        }
      }
    });
    this._super.included.apply(this, arguments);
  }

Seems the options are not loaded:

https://github.com/salsify/ember-css-modules/blob/25b1872521a708eea0bf6af9373accf572c62079/packages/ember-css-modules/index.js#L59

this.app is undefined, so is this.parent.options

FabHof commented 4 years ago

@dfreeman Could I get a quick status update on this? Is there currently any development in progress? I could try to dig into the code and find a fix, if you need help. But of course I don't want to get in the way of anything.

dfreeman commented 4 years ago

Hi @FabHof, thanks for your patience on this! After attempting several different approaches with this, I don't think there's a path forward that allows addons to continue setting their options like that in included — because of how component/template colocation works, we now need to figure out the options by the time template preprocessing is configured, which happens before included is invoked.

I've opened #186, which improves the awful error message you originally reported at the top of this thread, and also updates the guidance in the virtual modules documentation to use the setupPreprocessorRegistry hook rather than included. That change should be backwards compatible, i.e. it will work with older versions of ECM as well as going forward. In light of that, I'd suggest trying something along these lines in your addon's index.js:

  setupPreprocessorRegistry(target) {
    if (target === 'parent') {
      let parentOptions = (this.app || this.parent).options || {};
      let config = parentOptions.myAddon;

      this.options = Object.assign({}, this.options, {
        cssModules: {
          virtualModules: {
            colors: {
              primary: config.primary || '#a55176'
              // some more colors
            }
          }
        }
      });
    }
  }

That should allow you to update the options for your addon's ECM config before it's read.

https://github.com/salsify/ember-css-modules/pull/153#discussion_r332023228 has a brief thread around whether to consider this timing update a breaking change from a semver perspective. I'd love to avoid gating Embroider + colocation support behind a major upgrade for folks, but it's hard to know how often people are doing things like this in private packages that may get burned by the change (even if this timing technically isn't strongly-defined).

FabHof commented 4 years ago

Thanks @dfreeman for the good explanation. Sadly the workaround does not seem to work for the current 1.3.0-beta.1, it works for 1.2.1 however. Should it work for the current beta?

dfreeman commented 4 years ago

@FabHof it should! 🤔

Is the addon in question publicly available? That approach worked fine in the dummy addon I tested it out with.

FabHof commented 4 years ago

@dfreeman I created an example that does not build (at least for me): https://github.com/FabHof/ember-css-modules-addon-test

dfreeman commented 4 years ago

Thanks @FabHof, that's super helpful! I think I was watching for the wrong hooks to be hit in my own local testing 😅

In the end I think I've come up with an approach to restore the previous timing for computing options. I've updated #186 to include that change and am verifying the test suite continues to pass, but hopefully should have a release for you that fixes everything that's come up in this thread within the next day or so 🙂