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

components in v2 addon's app-js using index files broken #1835

Open
simonihmig opened this issue Mar 4, 2024 · 5 comments
Open

components in v2 addon's app-js using index files broken #1835

simonihmig opened this issue Mar 4, 2024 · 5 comments
Labels
bug Something isn't working

Comments

@simonihmig
Copy link
Collaborator

Given index-modules based file layout for colocated component files as supported by RFC481 in a v2 addon:

src
└── componennts
    └── fancy-button
        ├── index.ts
        └── index.hbs

And generating the package's app-js by the rollup plugin provided by @embroider/addon-dev:

{
  "ember-addon": {
    "version": 2,
    "type": "addon",
    "main": "addon-main.cjs",
    "app-js": {
      "./components/fancy-button/index.js": "./dist/_app_/components/fancy-button/index.js"
    }
  }
}

when building an app consuming the addon using Embroider v3.4.5 and "optimized" settings, it would fail with:

Module not found: Error: Can't resolve '#embroider_compat/components/fancy-button' in '[...]/test-app'

It does work work with either

  • classic build w/ eai
  • Emboider v2.x
  • Embroider v3.4.5 w/ "safe" mode (no static components)

It will also work in Embroider v3.4.5 "optimized" when changing the app-js's left side to not use index modules:

-"./components/fancy-button/index.js": "./dist/_app_/components/fancy-button/index.js"
+"./components/fancy-button.js": "./dist/_app_/components/fancy-button/index.js"

So we could fix this by exposing the app-js not using index modules, but since index-modules are officially supported by RFC481, it looks like a bug in Embroider?

@simonihmig simonihmig added the bug Something isn't working label Mar 4, 2024
@NullVoxPopuli
Copy link
Collaborator

I think this can be customized via:
https://github.com/embroider-build/embroider/blob/main/packages/addon-dev/src/rollup-app-reexports.ts#L11

addon.appReexports(['**/*.js'], { mapFilename: (filename) => {
  if (filename.includes('/components/') && filename.endsWith('index.js')) {
    return path.dirname(filename) + '.js';
  }
} }),

does that work?

See also prior discussions:

tl;dr: we opted not to resolve this at the resolver layer, since it's an explicit app-js and package.json exports resolution configuration that causes the behavior you're seeing.

@mansona
Copy link
Member

mansona commented Mar 4, 2024

So this was recently fixed on main and it's likely just a bug in the component resolver. I could see some value fixing this on stable but I think we have diverged a bit much to be able to simply backport the change 🤔

@simonihmig
Copy link
Collaborator Author

@mansona are you able to point me to where this was fixed on main?

@simonihmig
Copy link
Collaborator Author

I think this can be customized via:

Yeah, I know this workaround exists. But the question in this issue was more where to properly fix this? Is this a bug in Embroider (seems so, as Chris confirmed), or should we bake that behaviour into the rollup plugin? I'd rather not want to introduce this workaround everywhere in user-land code, since index-modules is a supported thing.

@NullVoxPopuli
Copy link
Collaborator

NullVoxPopuli commented Mar 4, 2024

since index-modules is a supported thing

Not in ESM node 😅 (which is a bit of an oversight for co-location -- I also learned this by running in to broken stuff (and as it turns out, this is what package.json#exports follows)).
I think the fix @mansona is talking about is the loose mode resolver rules, which had index behavior fixed here: #1794
Commented here: #1826 (review)
Actual fix here: https://github.com/embroider-build/embroider/pull/1794/files#diff-61967680c203e14cb9a3b2c252caca7d6b0ee9b285991dba6b728f6c54112a12R571

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants