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

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

Closed
FabHof opened this issue Dec 13, 2019 · 9 comments · Fixed by #186

Comments

@FabHof
Copy link

FabHof commented Dec 13, 2019

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
Copy link
Contributor

buschtoens commented Dec 16, 2019

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.

function resolveExternalPath(importPath, options) {
let baseIndex = importPath[0] === '@' ? importPath.indexOf('/') + 1 : 0;
let addonName = importPath.substring(0, importPath.indexOf('/', baseIndex));
let addon = options.parent.addons.find(addon => addon.name === addonName);
if (!addon) {
throw new Error(`Unable to resolve styles from addon ${addonName}; is it installed?`);
}

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
Copy link
Member

🤔 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
Copy link
Author

FabHof commented Dec 16, 2019

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:

let includerOptions = this.app ? this.app.options : this.parent.options;

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

@FabHof
Copy link
Author

FabHof commented Feb 17, 2020

@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
Copy link
Member

dfreeman commented Apr 7, 2020

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.

#153 (comment) 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
Copy link
Author

FabHof commented Apr 7, 2020

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
Copy link
Member

dfreeman commented Apr 7, 2020

@FabHof it should! 🤔

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

@FabHof
Copy link
Author

FabHof commented Apr 7, 2020

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

@dfreeman
Copy link
Member

dfreeman commented Apr 8, 2020

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 🙂

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