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

A robust way to resolve imports. #123

Open
AnatoleLucet opened this issue Apr 25, 2020 · 7 comments
Open

A robust way to resolve imports. #123

AnatoleLucet opened this issue Apr 25, 2020 · 7 comments
Labels
discussion enhancement New feature or request

Comments

@AnatoleLucet
Copy link
Collaborator

AnatoleLucet commented Apr 25, 2020

If you already know a package that is able to resolve pretty much any kind of imports (e.g. Typescript import aliases, Babel module resolver, etc...), then you don't have to read the following. Simply comment it out because I couldn't find one myself.

After seeing the recent opened issues (#122, #113, #102, #115, #107, and probably more soon...), I've noticed that we clearly need a more robust way to resolve imports. I'm thinking about a package made for Destiny but, that could be used by anyone else for any kind of projects. I assume there would be some interest for such a package (a simple utility in Destiny's engine wouldn't permit that). A bit like the relation between Svelte and code-red...

Here's a non-exhaustive list of paths that this package would need to handle:

And I'm 100% sure that there's a lot more, so if you know any other way to tweak the "vanilla" import path, please comment it out! 🙏

cc @benawad, @sQVe.

Edit:
I've created the repo: https://github.com/AnatoleLucet/Deema
I'll start working on it once I have the time to.

@AnatoleLucet AnatoleLucet added enhancement New feature or request discussion labels Apr 25, 2020
@AnatoleLucet AnatoleLucet changed the title A robust way to resolve import. A robust way to resolve imports. Apr 25, 2020
@benawad
Copy link
Owner

benawad commented Apr 25, 2020

I agree

I don't think resolving Dynamic imports is possible, unless we eval their code?

Maybe we just detect dynamic imports and show a warning that we can't handle them

@AHBruns
Copy link

AHBruns commented Apr 26, 2020

It seems that the best approach to this would be to define a DestinyLinkParser interface and create a few implementations for the initial languages we want to support (not sure what these are as of now). Then, as we add new languages, we just have to write a new parser that implements the interface. I bet this would make community contributions much easier to.

To be clear, this interface approach doesn't preclude making a universal import resolver package, but I think it'd be good to decouple the two, at least on Destiny's side so others can use their own resolvers with Destiny for whatever language they're into.

Also, I came across a pretty nasty bug today where comments on the same line as an import hide it from Destiny causing all sorts of erroneous unused file errors and and a malformed output file structure. I was going to make an issue, but it seems like all bugs of this flavor should probably just get attached to this till we have a robust resolver solution in place.

@AnatoleLucet
Copy link
Collaborator Author

AnatoleLucet commented Apr 26, 2020

for the initial languages we want to support (not sure what these are as of now)

JS-like languages (e.g. Typescript, Flow...). We're also trying to support css pre-processors, but it doesn't work that well for now (#114).

@danielpza
Copy link
Contributor

danielpza commented Nov 30, 2020

There's some support already on https://github.com/AnatoleLucet/Deema for typescript's tsconfig.json, I tried it out with destiny and seems to work:

modified   src/index/shared/customResolve.ts
@@ -1,4 +1,5 @@
 import resolve from "resolve";
+import { generateResolver } from "deema";
 
 const extensions = [
   ".js",
@@ -19,7 +20,10 @@ const extensions = [
 /** Resolve with a list of predefined extensions. */
 export const customResolve = (id: string, basedir: string) => {
   try {
-    return resolve.sync(id, { basedir, extensions });
+    const resolver = generateResolver(basedir);
+    const resolved = resolver(basedir + "/file", id);
+    const final = resolve.sync(resolved, { basedir, extensions });
+    return final;
   } catch (err) {
     return null;
   }

Some adjustments on the import regexp is needed though, some imports like import { ... } from '@/shared/components.js' were not working because is not a relative path

@AnatoleLucet
Copy link
Collaborator Author

@danielpza If you'd like, I can release an initial version of deema with what you did so we can implement it in destiny.
It'd help to get some users' feed back.

@danielpza
Copy link
Contributor

@AnatoleLucet yeah, that would be great.

@AnatoleLucet
Copy link
Collaborator Author

Done https://www.npmjs.com/package/deema

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants