Allow modules to import themselves - #1777
Conversation
lukastaegert
left a comment
There was a problem hiding this comment.
Thanks, that was really quick! I agree with your assessment that since we throw maximum call stack errors for larger cycles, we can just as well remove this one test until we have a more general solution.
I would just suggest to change the tests to be a little more realistic, see other comment. Otherwise I approve the change.
| @@ -0,0 +1,3 @@ | |||
| import me from './main'; | |||
There was a problem hiding this comment.
I think we should change this example to something a little closer to an actual use-case. How about something like this:
// main.js
import {run} from './lib.js';
run();
// lib.js
import * as lib from './lib';
export const run = () => lib.log();
export const log = () => console.log('log');? This would document a little better as to why someone would even want to do that. And we can also remove the checked warning from the config which I think is probably rather distracting to the unsuspecting reader.
And maybe we could also make the "empty" test a little more interesting by basically doing the same but without a side-effect in the secondary function. Then we can show that tree-shaking works properly as well in the case of self-imports.
What do you think?
Update: As we actually want to check an export in the test, maybe something like this makes more sense for the non-empty case:
// main.js
import * as lib from './main.js';
export const getExported = () => lib.getOutput();
export const getOutput = () => 'exported');and then checkt that getExported() returns 'exported' in the test.
|
Thanks for the suggestions, I've pushed some adjustments along these lines. Let me know how that seems. |
|
(Note the build failure here seems unrelated) |
As discussed in some detail at #1055 (comment) this case exactly follows the specification as Rollup works currently, including the TDZ errors on bindings.
I've removed the
export * from 'self'test entirely here though, as theexport *code doesn't seem to handle cycles correctly (hits maximum stack). Cycles are supposed to throw for that case though, so that could be a further improvement, and would happen anyway right now in anyexport *cycle in Rollup (a module importing itself being the smallest cycle).Would be another step towards spec compliance, just let me know any feedback.
Resolves #1055.