Does not get correct dependencies when mixing es6 and cjs import syntax about node-precinct HOT 9 CLOSED

@pcardune I added an option es6.mixedImports that will chain the es6 and cjs detectives. See the usage docs for the syntax.

You can get this with a fresh install of madge, but I'll still bump precinct within dependency-tree.

Thanks again for suggesting the feature. Let me know if you run into any issues.

from node-precinct.

Fair point @pcardune! The goal of precinct is zero configuration, but i like the opt-in perf regression for transitionary codebases. Will work on that and support the option all the way up to madge.

from node-precinct.

Thanks so much for the details @pcardune! I'll pull down that repo and use it as a performance testbed for the change to run both the commonjs and es6 detectives for CJS or ES6 files. I'll also continue to think through some alternative solutions.

I'm a big fan of what code.org is doing, so I'm happy to work on a solution. I'll keep you posted on the progress; though I foresee it taking a couple of nights of work to land on a fully tested solution that achieves a lot of the goals I've mentioned earlier.

@pahen any additional thoughts on a path forward that doesn't introduce performance regressions?

from node-precinct.

Hey @pcardune! Thanks for filing the issue! Out of curiosity, for what purpose are you using precinct?

I can think of a few ways of approaching the mixed import case:

1. You could use Lebab's commonjs transform to turn CJS requires into ES6 imports https://github.com/lebab/lebab#safe-transforms.
2. I could look into extending detective-es6 with the functionality detective-cjs (since mixed codebases are likely until we have a proper es6 loader). This is hairy since I need to juggle a few things: preserving the separation of import concerns, avoiding double traversals of an ast, and finding a sane runtime extension point to combine the checks for es6 and cjs.

While I'm happy to make this lib more robust, I'd prefer to find a decent workaround until more folks request support for this use case. Thoughts?

from node-precinct.

I'm actually not using precinct directly. I'm using madge to generate svg dependency graph images. madge uses dependency-tree, which uses precinct, which lead me to this repo :)

I'm also thinking about using dependency-tree to power a linter that enforces rules about how modules in different directories can depend on each other.

I will check out lebab and get back to you. I previously tried using some transforms for jscodeshift to convert require() calls, but alas, we have some nested require() calls and the transform doesn't handle them properly :( :(. Hopefully lebab works better...

from node-precinct.

@mrjoelkemp

Unfortunately lebab does not manage to figure out when to use

import * as foo from './foo-module'

instead of

import foo from './foo-module'

so converting all the code would require a lot of manual work :( :(

Here is an example of a file that I am working with: https://github.com/code-dot-org/code-dot-org/blob/staging/apps/src/applab/applab.js

from node-precinct.

Hmm, that was a tricky one. I suppose it would involve a lot of work and changes to support mixed import syntaxes @mrjoelkemp ? I'm afraid I can't think of any easy fix for this that doesn't introduce performance regressions :(

from node-precinct.

FWIW, I would be willing to take performance regressions to make this work. Perhaps it would be possible to add an option that explicitly enables mixed import syntaxes, which third parties could then choose to enable/disable depending on their needs?

from node-precinct.

Can we get the same mixed imports functionality for TS? I'm in a codebase that's migrating from JS to TS, and we have some cases where we still need to use CJS.

from node-precinct.