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

named/default can't resolve local package.json #387

Closed
billyjanitsch opened this issue Jun 21, 2016 · 9 comments
Closed

named/default can't resolve local package.json #387

billyjanitsch opened this issue Jun 21, 2016 · 9 comments
Milestone

Comments

@billyjanitsch
Copy link

billyjanitsch commented Jun 21, 2016

// test/myTest.js
import {foo} from '..'

// src/index.js
export const foo = 5

// lib/index.js
exports.foo = 5

// package.json
// ...
"main": "lib/index.js"
// ...

This throws foo not found in '..' even though node is correctly able to resolve the path. I don't see a clear way to add an import/ignore exception here -- is this possible?

@benmosher
Copy link
Member

Strange. As outlined, that should work.

What resolver are you using (if any)?

@billyjanitsch
Copy link
Author

Sorry, I forgot to explain the context: lib is the compiled (by Babel) directory -- the idea is that I want my tests to import the compiled code instead of the source. I've updated the example above to be more clear.

I know that the plugin won't be able to verify the import from myTest, since the respective export has been compiled to CJS. I'm just asking if there's a way to add an import/ignore rule that says "ignore the main field of all package.json files" (since this field should point to a CJS file). The default node_modules doesn't work because this package.json is local.

@benmosher
Copy link
Member

Could you use jsnext:main and point it to the original ES6 source? https://github.com/rollup/rollup/wiki/jsnext:main

@billyjanitsch
Copy link
Author

billyjanitsch commented Jun 22, 2016

That works if the source is ES6, but not if it's ES2016+ (since other tools that follow jsnext:main may break with ES2016+ syntax), and I don't think it's worth creating an intermediate ES2016+ > ES6 build that jsnext:main can point to just to get this working.

As a workaround, adding lib to import/ignore works, but I'm wondering whether this should be the default behavior. Shouldn't the default resolver never try to parse exports of files that it reaches via the main field of a package.json? (i.e. won't such files always contain CJS rather than ES6 exports?)

Thanks for spending time discussing this, by the way!

@billyjanitsch
Copy link
Author

billyjanitsch commented Jun 22, 2016

I looked into the code and it looks like the behavioral change I'm describing might be as simple as deleting the main key before this conditional assignment. Just kidding, that would presumably break no-unresolved.

In any case, I'm happy to look into this and submit a PR if you agree with the change!

@benmosher
Copy link
Member

Ah, I see what you're saying.

I do have an outstanding item to auto-ignore CJS modules, which I think would have the effect you're looking for?

@benmosher
Copy link
Member

Shouldn't the default resolver never try to parse exports of files that it reaches via the main field of a package.json? (i.e. won't such files always contain CJS rather than ES6 exports?)

Just taking a second to think this through. I think we shouldn't ignore main modules for packages.

ES6 modules will land in Node soon, and folks will have packages published to npm mere moments later. 😄

I think #270 should solve this (just redefined it). If the resolved module is without imports/exports, it will be ignored. So you won't get linting for real errors, but you won't get spurious errors, either.

@benmosher benmosher added this to the 2.0.0 milestone Jul 1, 2016
@benmosher
Copy link
Member

Closing as #270 is fixed in v2.

@billyjanitsch
Copy link
Author

#270 looks like a great solution to me. Thanks a lot!

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

No branches or pull requests

2 participants