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

Cannot find module error using Dynamic and Nested imports in the same application #10643

Closed
filipenevola opened this issue Jul 22, 2019 · 5 comments

Comments

@filipenevola
Copy link
Collaborator

@benjamn I'm getting the same problem of this issue #10496 in a big project that uses heavily local Meteor packages, nested imports, and dynamic imports. I think the root cause is this mix between nested and dynamic imports, I don't think local Meteor packages is affecting it.

The error is like Cannot find module '/node_modules/meteor/pathable-ui/containers/helpers/permissions/with-is-allowed.jsx

Looking into Sources tab of Chrome Dev Tools I see the module mentioned in the error being mentioned many times on static imports calls but its content is not there and that leads to this error.

Maybe what is happening is that this module is also requested by other modules that are used with nested imports and then somehow Meteor builder is avoiding it on the static bundle.

I already restart my application, cleaned meteor/.local, etc then I believe my case is not a cache problem.


Background: we have a bunch of widgets that are React components that represent each piece of our UI and to avoid evaluating all these modules at startup time we use nested imports in the React component that render these widgets into a page. Now I'm trying to also dynamic load some of these widgets components that are not loaded frequently to reduce bundle size.

The hard part is that the error that I'm getting is not directly related to the widget that I'm dynamically loading now.

Anyway, I know you need a reproduction and I'll try to build one but in the meantime maybe this description can ring a bell of some potential gap in Meteor implementation of dynamic and nested imports.

@benjamn
Copy link
Contributor

benjamn commented Jul 22, 2019

Happy to look at a reproduction, even if that means sharing a private repo (if you're comfortable with that).

The module system should be treating nested imports the same as top-level imports when scanning the dependency graph, so I would be surprised if that's the source of the problem, though I could imagine the dependency scanner somehow missing a nested static import (which would be a bug), so the module might appear to be imported only dynamically.

If you can fix (or at least change) the behavior by temporarily hoisting the nested imports to the top level, I think that would lend additional evidence to the theory.

@filipenevola
Copy link
Collaborator Author

Sorry @benjamn for the long delay, I was on vacation, back now 😄

We are having a hard time creating a reproduction in a small project. I tried before my vacation and also @Gywem tried some weeks ago.

Can we screen share? If not, I can share the code with you but it will take more time than screen sharing as you will need to setup many modules and tools to run our system properly.

I don't think you will be able to figure out just by looking at the code due to many layers and packages 😞.

@filipenevola
Copy link
Collaborator Author

filipenevola commented Sep 5, 2019

@benjamn I have good news.

I'm still confirming with more cases but so far everything is working well. I want your feedback then I can proceed with a PR.

Anyway, the cases that were breaking in my last changes on Pathable code are not happening anymore after one change on Meteor import-scanner.

Let me summarize to you what I found and what I change, I came to this conclusion after studying and debugging Meteor code for a few hours today morning:
1 - Meteor scans every module and then creates a list of missing modules;
2 - Then handles these missing modules avoiding to handle the same module twice.

The way Meteor was avoiding to handle the same module twice was not considering that the first time this module could be missing in a parent using dynamic import but in the second time the parent could be a static import. Because of that Meteor was not able to add every static module on the static bundle, this was happening for NPM modules and also for local code. In case of Pathable NPM modules are included via Meteor package then I'm not sure if that was also happening for NPM modules included directly. And just for the record I also got this problem in project outside Pathable then it's not related to Pathable architecture because the other project is a lot smaller and it's just a plain Meteor project without any local package.

My change was to replace this code

Object.keys(newlyMissing).forEach(id => {
if (has(previousAllMissingModules, id)) {
delete newlyMissing[id];
} else {
ImportScanner.mergeMissing(
previousAllMissingModules,
{ [id]: newlyMissing[id] }
);
}
});

with this below

const dontNeedToScanAgain = has(previousAllMissingModules, id) 
&& !isHigherStatus(getParentStatusFromInfo(newlyMissing[id]), getParentStatusFromInfo(previousAllMissingModules[id]));

if (dontNeedToScanAgain) {
  delete newlyMissing[id];
} else {
  ImportScanner.mergeMissing(
    previousAllMissingModules,
    { [id]: newlyMissing[id] }
  );
}

I also refactored a little bit the code to make this new code easier

const Status = {
  NOT_IMPORTED: false,
  DYNAMIC: 'dynamic',
  STATIC: 'static',
};

const importedStatusOrder = [Status.NOT_IMPORTED, Status.DYNAMIC, Status.STATIC];
function getParentStatusFromInfo(info) {
  return info.some(entry => !entry.parentWasDynamic) ? Status.STATIC :
    Status.DYNAMIC;
}

function isHigherStatus(status1, status2) {
  return importedStatusOrder.indexOf(status1) >
    importedStatusOrder.indexOf(status2);
}

function setImportedStatus(file, status) {
  if (isHigherStatus(status, file.imported)) {
    file.imported = status;
  }
}

Let me know if my understanding is correct then I'll submit a PR for your review 😉

filipenevola added a commit to filipenevola/meteor that referenced this issue Sep 5, 2019
… application meteor#10643

- fixes the problem
- adds temporarily some logs to help analyzing specific cases
filipenevola added a commit to filipenevola/meteor that referenced this issue Sep 5, 2019
… application meteor#10643

- fixes the problem for multiple entries for the same module
filipenevola added a commit to filipenevola/meteor that referenced this issue Sep 8, 2019
filipenevola added a commit to filipenevola/meteor that referenced this issue Sep 8, 2019
@idanwe
Copy link

idanwe commented Nov 2, 2019

@filipenevola @benjamn I have the same problem for at least 2-3 months in production, I actually never able to make a reproduction, so I will be more than happy to help try to create a reproduction if someone can help guide me.

So what is worth, the only way to "solve" it to users is to ask then to delete the "Site Data" but actually what is needed is to delete the IndexDB.

My app does quite heavy using of dynamic imports for local pages, mainly for code splitting based on routing in the client-side and internal modules/components and npm

@filipenevola
Copy link
Collaborator Author

@idanwe maybe this is a different issue or in your case, the order of navigation (fetching dynamic modules) is affecting.

If you want to try this now update your Meteor app to 1.8.2 release candidate 4 with this command
meteor update --release 1.8.2-rc.4

Let me know the results.

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

No branches or pull requests

3 participants