JorgenVatle/meteor-vite

NPM package is imported twice from different sources

Closed this issue ยท 10 comments

Hi, @JorgenVatle!

I found a subtle but quite critical issue. If some NPM package is imported in both the main app code and a Meteor package, it's imported from different sources. In the first case it's a file processed by Vite, while in the second case it's modules.js file generated by Meteor build system. This causes 2 problems:

  1. 2 versions of the same code are loaded on the client.
  2. If an NPM package has some kind of internal state, the app and the package operate on different states.

I created a repo to illustrate the issue. Here we have a NPM package which has a random value as its internal state. Meteor package and the app code receive different values.

0  console
1  app
2  package

Thanks for reporting this! ๐Ÿ™

I ran into similar issues when preparing a React example app. This appears to be the cause for incompatibility between Meteor's React packages and React processed by Vite.

Unfortunately I couldn't find any good solution at the time. Though I imagine we could set up a Vite plugin to point imports for these packages to the Meteor bundle instead of letting Vite compile them.

This appears to be the cause for incompatibility between Meteor's React packages and React processed by Vite.

This is also true for rdb:svelte-meteor-data. It has imports from svelte/internal - and that's how I discovered the issue.

Though I imagine we could set up a Vite plugin to point imports for these packages to the Meteor bundle instead of letting Vite compile them.

So the Vite plugin should:

  1. Detect all imports from NPM packages inside Meteor packages.
  2. If there is the same import in main code, point it to the Meteor bundle.
    Is that correct?

Yeah, that's what I'm thinking would be the best approach to it.

Sorry for getting back to you so late on this.

In that case, it should be also taken into account that Meteor package can use Npm.depends() functionality. So the algorytm should be something like:

if (app and package use the same NPM_package) {
    if (package has NPM_package in Npm.depends) {
        if (versions in NPM.depends and package.json are the same) {
             import from Meteor bundle
        }
        else { compile with Vite }
    }
    else { import from Meteor bundle }
}
else { compile with Vite }  

Hello @JorgenVatle.

Do we understand correctly that this issue is not closed, since a solution has not yet been found?
We recently decided to test this - in the repo to illustrate we updated all dependencies to the latest versions, but this did not help.

Or maybe we need to make some settings that we may have missed?

Oh, hey, didn't spot your comment until now.

Resolving the issue was a little more complex than I had anticipated and would require a notable amount of refactoring if I recall correctly. Just haven't found the time to get started on that. Externalizing these packages with a custom Vite plugin has been the easiest way to deal with the issue so far. But that assumes that you know which packages are provided by Meteor of course. ๐Ÿ˜…

I'll have another crack at it later in the evening and get back to you if something comes up. ๐Ÿ‘

Hey, first of all thanks for the amazing work you do here!
I unfortunately have the same issue with react being imported twice, the workaround in the react example doesn't fix it since after the transformation the chunk is missing some other exports.

Did you already had a chance to look into it again? Is there maybe some other workaround I could try?

I have a fix in the works now. ๐Ÿ˜„

Though, since we still need to rely on data made available to us from the Meteor bundle. These npm packages could potentially be outdated or only partially included in the client bundle if Meteor is to do any of tree-shaking. I haven't had the chance to verify whether this is the case, but I suspect it's probably not something you have to worry about.

Either way, just to be safe, we'll make it something you need to opt in to for the time being.

You'll be able to do this on a per-import basis. The syntax will be something like the following:

// No changes here. 
// This will import from your node_modules like you would expect
import MyPackage from 'some-npm-package'

// If you have a Meteor package that already comes bundled with an npm package, the following
// syntax can be used to tell Vite to externalize this import request.
import MyPackage from 'meteor:some-npm-package';

You can import from both places if you find a use case for it. Bearing in mind your client bundle will include both versions

Bit on an update on this. It does indeed appear like Meteor is lazy-loading npm packages. In the final production bundle, it still might not be an issue as those imports should be picked up on. But in development, it's looking like we need to add explicit exports for each module you need into your Meteor client's mainModule.

With React, I'm not sure how sensitive it is to having multiple versions running. Like, if we pull in a library for React, and this library gets bundled by Vite and not Meteor, would React complain about that? Or a better way to put it, do React libraries ship with a version of React, or is that usually just a peer dependency?

If React libraries use their own versions of the React library, it's probably going to end up being a much better developer experience to simply publish a fork of the react-meteor-data package over npm instead of relying on the Atmosphere-published version. With this approach, we're probably also looking at a considerable improvement in production bundle times as well, since Meteor will no longer be responsible for compiling your React dependencies.

@durac @red-meadow @dispbd The latest release (meteor-vite@1.10.0 and jorgenvatle:vite-bundler@1.12.9) should resolve the issues you noted. Let me know how it goes. Please do re-open the issue if you're still encountering issues.

@durac I've updated the documentation for React as well as the React example app. Give it a shot and let me know if you run into any issues. ๐Ÿคž