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

Resolve accompanies to just the original import id #462

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

klaftertief
Copy link
Contributor

Maybe I misunderstood how combining multiple main files worked, at least I could not make it work for our use-case.
I really love how the feature is used by just adding multiple Elm modules as query params, this makes it super flexible.

We have four different Elm apps which should be compiled into one bundle. There are four different JS modules that import the combined Elm apps individually. So index-a.js, index-b and so on each import A.elm?with=B.elm&with=C.elm&with=D.elm. This is simplified, as the JS and Elm entry points might live at different directory levels. It worked at some cases, but adding the fourth Elm module to the list made it break. I believe this was caused by setting a wrong importer when walking the import tree in the original implementation. I do not know a lot about Vite's or Rollup's internals, but it made me wonder if it is actually needed.

In this PR I made it such that the path in each with param is relative to the originally required Elm module, A.elm in this case. So in the implementation I use a simple path.resolve to add each accompany to the list of compilable Elm modules. It is much simpler and is something I can understand. But I also have a feeling I'm missing something in the original implementation.

@hmsk
Copy link
Owner

hmsk commented Apr 26, 2023

Uh, looks really simpler! However, we need to rely on plugin context to resolve importing. The relative path may include some alias or magic words to work with other plugins.

But I'm curious about the problem you faced originally. Can you give me an easier way to reproduce that situation?

@klaftertief
Copy link
Contributor Author

Thanks for the quick response. I'll see if I can spin up a SSCCE...

Good to know about the context to resolve imports. But does it really apply to Elm modules? The path/id of the original Elm module (as imported from JS) is not touched, everything with-based is just handled by the Elm compiler and based on the original Elm module.

The relative path may include some alias or magic words to work with other plugins.

Can you give an example here?

@hmsk
Copy link
Owner

hmsk commented Apr 26, 2023

what if vite's config has alias setting like:

  resolve:{
    alias:{
      '@' : path.resolve(__dirname, './src')
    },
  },

We'd expect import { Elm } from '@/Description.elm?with=@/AnotherDescription.elm' works.

@klaftertief
Copy link
Contributor Author

Thanks, yes, this would totally not work with this PR :-)
Converting it to a draft.

@klaftertief klaftertief marked this pull request as draft April 26, 2023 05:56
@hmsk hmsk force-pushed the main branch 11 times, most recently from b3d2607 to df9eedf Compare April 17, 2024 09:32
@hmsk hmsk force-pushed the main branch 6 times, most recently from b5c115a to 45186cb Compare April 27, 2024 05:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants