Closed iahu closed 3 years ago
hi @trotzig can you take a look on my PR.
Hi @iahu! Thanks for the contribution! I'm a little hesitant to merging this as-is though because of the memory and performance footprint. Can you make it opt-in through a configuration option? It would also be good to have some tests written for the new behavior.
Hi @iahu! Thanks for the contribution! I'm a little hesitant to merging this as-is though because of the memory and performance footprint. Can you make it opt-in through a configuration option? It would also be good to have some tests written for the new behavior.
yes, i notice the performance issue too. maybe i can make a configuration option for it.
how about lazy find exports, but not batch find by expand modules from package.json ?
Aside from performance, since modules may have side effects when they are required, I'm concerned about the safety of this change.
CommonJS loads module in a stand alone sandbox,so scope is safe. And browser module have no fs module, so, side effects mostly may be network, maybe we can proxy the xhr object to prevent it.
Honestly, I also worry about the side effects. Maybe we can make it as a user configurable.
在 2021年4月20日,05:54,Joe Lencioni @.***> 写道:
Aside from performance, since modules may have side effects when they are required, I'm concerned about the safety of this change.
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.
i doubt that tsserver/lsp will face the some side effect problem. if a module has side effect, it can also happen at browser runtime or node.js runtime.
a public module should not be eval, if so, it’s hard to defence it. personally, i want a more smarter tool to help me mange importing, and i can accept native require as a optional for me.
some modules export values with condition expressions, like UMD or production/development version. use native
require
function to get exports from that kind modules, like React and lodash.