Implement imports

[izaera]

Now that we have explicits exports (#516) we need to be able to import packages from a different OSGI bundle.

The "problem" is that there's something incorrect with the imports in bundler 2.x that we can fix for bundler 3.x but at the cost of introducing a big interoperability burden that will be with us until everybody migrates everything to bundler 3.x.

Let's see the problem. Imagine you create the react-portlet and react-provider projects (both at version 1.0.0) where the second exports the React framework (version 16.12.0) so that the first one imports it.

This would lead to the following module defined in the AMD loader:

react-provider$react@16.12.0/index

Which would be imported by react-portlet by means of the .npmbundlerrc.

This looks good, but pay attention to the name of the module: it contains React's version (16.12.0) but not react-provider's version (1.0.0). Thus, if we deploy another version of react-provider (which is legal in OSGi) there will be a collision if it also contains react@16.12.0.

We have never seen any problem related to this because the scenario is quite strange and even if it happened, the loader would choose one of the two react@16.12.0 copies and, if both are the same (which is expected), nothing would happen.

However it is tempting to fix it for bundler 3.x, not only to avoid the error, but to simplify how the project is configured and built a lot (see next comment).

[izaera]

In bundler 2, for imports to happen configuration is quite complex. In particular, you need to specify react's version two times in react-portlet: one in package.json and another in .npmbundlerrc, like this:

package.json

{
  "dependencies": {
    "react": "^16.0.0"
  }
}

.npmbundlerrc

{
  "config": {
    "imports": {
      "react-provider": {
        "react": "^16.0.0"
      }
    }
  }
}

Note that in this case we use the same semver expression in both files, but it could be different because the first one is just used by the project during build time and the second one during runtime. You can even remove the dependency from the package.json file and nothing will fail during runtime!

This leads to several strategies to handle project and imports versions which adds complexity and separates from the standard expected frontend development practices (though it is obvious that we will need some deviation as the imports are not a standard feature).

In an ideal world we would like to specify everything in package.json and simply tell the bundler where to get dependencies from during runtime. And then leave everything else to the bundler/platform.

We'll see how this can be achieved in bundler 3.x in the next comment.

[izaera]

Imagine if we could be able to configure imports like this:

package.json

{
  "dependencies": {
    "react": "^16.0.0",
    "react-provider": "1.0.0"
  }
}

.npmbundlerrc

{
  "imports": {
    "react": "react-provider"
  }
}

Wouldn't it be much simpler?

Doing it this way we would be able to enforce version constraints during build time because we would be able to compare react-provider's React version with the one in react-portlet and see if they are compatible, failing the build otherwise.

The only limitation of this model is that react-provider must exist as a npm package. In the times of bundler 2 we weren't able to satisfy that because the Portal OSGi modules were not true npm packages, but now that we use yarn for the build this limitation doesn't hold any more so I guess we could go this way... 🤔

Also, if we iterate a bit over this idea, we can get rid of the dependency with react in react-portlet given that react-provider already provides it transitively. In that case we wouldn't even need to check anything, because the project would be built with the same version it is run.

[izaera]

So, if we decide to go that way, we would configure something like:

package.json

{
  "dependencies": {
    "react-provider": "^1.0.0"
  }
}

.npmbundlerrc

{
  "imports": {
    "react": "react-provider"
  }
}

But note how, in this case react-provider has a ^ like semver constraint which is the usual way to go when we want a project to be automatically updated. But, because this semver constraint is resolved during runtime, it may lead to an update of react version during runtime when the project has been built with a previous one (think what would happen if the project was built with react-provider@1.0.0 pulling react@16.10.0 but then, in the portal, someone deploys react-provider@1.1.0 which pulls react@16.12.0).

Do we really want that? 🤔 I would say we can go that way because if the user doesn't want that to happen he can always use 1.0.0 instead of ^1.0.0, but I admit deciding one thing or the other may lead to a less error prone platform.

[izaera]

Just to be exhaustive, the way this would be implemented would be, instead of react-provider exporting a module named react-provider$react@16.12.0/index it would simply export something like react-provider@1.0.0/$package-export$/react.

With this configuration, nobody needs to specify react-provider's react version any more because it is unambiguous: knowing the version of react-provider, the version of react can be derived without problem.

And because the current platform already resolves semver constraints, if react-portlet specifies react-provider@^1.0.0 it would resolve to react-provider@1.1.0, react-provider@1.2.0 and so on automatically during runtime.

Also, the setup would be much more simple because all deployed bundles would look like projects without any third party dependency. This is because react-provider@1.0.0/$package-export$/react looks like an internal module of react-provider to the outside world, not as a third party dependency like it did in bundler 2 architecture. This means that if everything was expressed like that (bundler 3.x model), we could get rid of a lot of code in the portal too.

The only downside is that if we wanted to interoperate bundler 2 and bundler 3 OSGi bundles we would need to specify if what we are importing has bundler 2 like exports or bundler 3 which looks quite ugly 😞.

The same would happen with exports, though it will be less problematic because we can just export things both bundler 2 and 3 way on user's demand.

[wincent]

Some random thoughts on this.

The scenario you're trying to address here is so vanishingly unlikely (deploying two different versions of an OSGi module containing the same version of a dependency), and the likely consequences so benign, that we should make sure that our response to it is proportional. In other words, I think your arguments in favor of making things "simpler" are more persuasive than the one about handling the edge case.

Also, if we iterate a bit over this idea, we can get rid of the dependency with react in react-portlet given that react-provider already provides it transitively.

I am not a fan of making transitive dependencies hidden like this. Explicit is good. As a rule of thumb, I believe that if you have an import 'thing' in your source code, then there should be a thing in your package.json saying what version you expect. Now, if you want that version to be super loose because it will make your life easier that's fine by me, because then you're declaring "I need react, and I am happy to go with the version supplied by react-provider":

package.json

{
    "dependencies": {
        "react": "*",
        "react-provider": "^1.0.0"
    }
}

The other idea we talked about elsewhere was that these imports-based dependencies are technically peerDependencies and should be declared as such:

package.json

{
    "dependencies": {
        "react-provider": "^1.0.0"
    },
    "peerDependencies": {
        "react": "*"
    }
}

Doesn't exactly meet your goal of "simple" but it is very precise.

In any case, no matter what we do, I like the idea of simplifying what's in the .npmbundlerrrc.js:

.npmbundlerrc.js

module.exports = {
    imports: {
        react: 'react-provider',
    }
};

And if, for some reason, you wanted to select from multiple possible versions of the provider:

module.exports = {
    imports: {
        react: ['react-provider', '^3.0.0'],
    }
};

or some similar format: an object, a string ('react-provider@^3.0.0') etc.

I haven't thought as much as you about this, but I also wonder if we could streamline what seems to be the most common case:

I want OSGi modules A, B, C to depend on common code in shared module D rather than duplicating it.

Rather than catering to the more complex case which I am not even sure is one that people run into in the real world:

I have different versions of a package in modules A, B (etc), and from modules X, Y, Z I want to be able to grab a specific version from one of A, B, C...

If what you mostly care about is the first case, then there could be something in the metadata of react-provider that declares that it is a "provider" module, and that would mean that any project that had react-provider in a dependencies could be assumed to want to import the provider's exports rather than having their own. That would be "zero-config" in a sense (other than adding react-provider to your package.json), and you'd only have to provide settings in an .npmbundlerrc.js if you wanted to step out of that common use case.

[izaera]

I like the idea of putting something in react-provider so that react-portlet knows it exports react. Then, the bundler, by default, would import react from react-provider unless the user configures something in .npmbundlerrc to opt-out.

In cases where react-provider is not available through the npm registry, the developers can resort to using yarn or lerna (even npm link 😱) to be able to make react-portlet depend on react-provider.

[izaera]

Regarding how to express the dependency of react-portlet with react we can leave it up to the user, as it doesn't matter (for the bundler) if you omit it, add it as a peerDependency or semver contrain it to "*" in dependencies.

There's one corner case, however, that we should probably check (while bundling). It is possible that the user may configure something like:

package.json

{
  "dependencies": {
    "react": "16.0.0",
    "react-provider": "1.0.0"
  }
}

In this case, npm would pull two react copies: 16.12.0 and 16.0.0, putting the first one in the node_modules subfolder or react-provider and 16.0.0 in the project's node_modules.

In the case of bundling that would equate to not importing react from react-provider but bundling react 16.0.0 locally into the project and using it (instead of importing).

However, we should probably warn the user because it smells a lot like an error, not something really wanted. In addition we should add some way to tell the bundler that we really want it that way in case the user really configured that on purpose (probably the imports opt-out explained in the previous comment would be enough).

[izaera]

It looks like we have the solution to the imports/exports mechanism during build time.

Still, we need to decide how modules will be exported during runtime, i.e., like in bundler 2 (react-provider$react@16.12.0/index) or the new way (react-provider@1.0.0/$package-export$/react).

We said that conflicts because of the react-provider namespace not containing the version looks very improbable and even if they happen nothing would break. The situation in bundler 3, however, is a bit different because not everything is exported, just what the user configures, and everything else is bundled with webpack in an opaque .js file.

Nevertheless, if someone exports the same entry points in two different versions of the same provider, it should work equally well because nothing would be missing (as each entry point has everything it needs inside its webpack bundle).

There could be cases where the internal structure of the webpack bundling would be different but it would still work and the only downside would be that debugging would be a bit tricky. I don't see this as a big problem, but it still poses the risk that someone has a setup where they want to deploy two versions of the same provider and insist in being able to decide which one to import.

Still, I don't see why someone would want to deploy two versions of the same provider and, if someone does, we may forbid it in the portal and say it is a limitation.

So, if we agree on this, we can use bundler 2 structure which has the big benefit of being backward compatible (even though it's a bit more complex).

[izaera]

One more thing we should think about is how to export modules in packages with multiple entry points (like lodash, which has one different module per API function).

I would say we can configure things like lodash/* in the exports section of the .npmbundlerrc file but I'm still worried about the performance because that would create one webpack bundle per lodash module plus its entry point for the AMD loader which looks like it's a much more inefficient setup than the one in bundler 2. However, it may not be the case, and maybe webpack is smart enough to make it faster than bundler 2 even in this case. If not, we would need to think some way to switch between the two type of builds.

[wincent]

However, we should probably warn the user because it smells a lot like an error, not something really wanted. In addition we should add some way to tell the bundler that we really want it that way in case the user really configured that on purpose (probably the imports opt-out explained in the previous comment would be enough).

One thing to bear in mind is that this sort of thing (matching version numbers) is already enforced in liferay-portal by SF, I think in JSONPackageJSONDependencyVersionCheck.

So, if we agree on this, we can use bundler 2 structure which has the big benefit of being backward compatible (even though it's a bit more complex).

Sounds reasonable to me. One thing though:

We said that conflicts because of the react-provider namespace not containing the version looks very improbable and even if they happen nothing would break.

I don't think that's strictly true — it's more accurate to say that "things are unlikely to break". It is certainly possible that the NPM module wonderful-package, does something that is supposed to be globally unique (like setting up a singleton registry inside its module closure), and when you have two supposedly identical copies of wonderful-package loaded somewhere, your registry that thinks it is a singleton and can safely do globally visible things like attaching stuff to window or document.body etc is actually unsafe because its assumption about being a singleton is invalid.

I would say we can configure things like lodash/* in the exports section of the .npmbundlerrc file but I'm still worried about the performance

I say try it and see what happens. 😂

[izaera]

One thing to bear in mind is that this sort of thing (matching version numbers) is already enforced in liferay-portal by SF, I think in JSONPackageJSONDependencyVersionCheck.

That's much better (for us). But I was thinking in people outside.

[wincent]

Yeah, I figured that (which is why I mentioned liferay-portal). Just pointing out that this might be reinventing a wheel.