Paths to external modules should not change when compiling to esm format

[mzinbergs]

• Rollup Version: rollup v1.18.0
• Operating System (or Browser): Ubuntu 18.04.1 LTS
• Node Version: 3.5.2

How Do We Reproduce?

With following config:

export default {
    input: 'main.js',
    output: {
		file:'out.js',
		format: 'esm',
    },
    external:['/ex1.js','ex2.js','.ex3.js'], 
}

And main.js as:

import { x } from "/ex1.js";
import { y } from "ex2.js";
import { z } from ".ex3.js";
import { stuff } from "./internal.js";

function doStuff() {
	stuff.value = "gorp";
	var xx = x(stuff);
	var yy = y(stuff);
	var zz = z(stuff);
}

doStuff();

Expected Behavior

After compilation ( rollup –config ) out.js should show the following:

import { x } from '/ex1.js';
import { y } from 'ex2.js';
import { z } from '.ex3.js';
...

Actual Behavior

After compilation ( rollup –config ) out.js shows the following:

import { x } from '../../../../../../../ex1.js';
import { y } from 'ex2.js';
import { z } from '.ex3.js';
...

[lukastaegert]

This is by design. When an external module is a relative or absolute path, that path is renormalized. This logic has been created to make sure that external imports can be deduplicated. Example

// main.js
import './lib/lib.js';
import { x } from './lib/external.js';
console.log(x);

// lib/lib.js
import { x } from './external.js';
console.log(x);

// output
import { x } from './lib/external.js';

console.log(x);

console.log(x);

So the idea is that you can mark any file in your module graph as external and then Rollup will still consolidate all imports from this file into a single import, which can be used to manually split up a module graph.

It could be argued that this should not happen for absolute paths as what will happen is what you just witnessed and I cannot imagine this is something people would want. Also it should be possible (but isn't) to circumvent this by using the plugin interface which is why I am marking this as a bug.

[lukastaegert]

Actually it is not really a bug but a mechanism to prevent paths on your local drive from leaking into the bundle. The logic is that if an import starts with a /, we assume it is the resolved path of a module and renormalize it into a relative path. This is necessary because during bundling, ALL paths are resolved to absolute paths.
The tricky question is how to actually keep an absolute path. I think this will require a new option and/or an extension to the plugin interface.

[lukastaegert]

Just to state it in a slightly different way, if you were to mark /Users/my-name/documents/my-file.js as external, it would definitely make sense to renormalize this to a relative path (and this is the intended pattern for plugins), but how would Rollup know this is to be handled differently from /my-file.js?

[mzinbergs]

A couple of notes about your comments:

There is no way that mangling an absolute path is something that was put there by design. That's crazy. Why would you want that? It's a side-effect of an algorithm designed to fix something else. It's a bug.

My understanding of “external” is that it prevents rollup from loading and evaluating the “external” module. It then becomes the executing context's responsibility to load, evaluate and execute the module. In order for it to do that it needs to know how to load the module. So the path is not there for rollup to use, it's there for the context executing the module to use. Don't modify it!

You don't need 'a mechanism to prevent paths on your local drive from leaking into the bundle'. Doing so assumes your users are so inept that they haven't figured out how to properly use a file system and don't know the difference between an absolute file path and a relative one. That's outrageous.

Resolving the paths and not duplicating the loading of “external” module is possible. Browsers do it. I tested a version of your code in two browsers and they didn't load the “external.js” module twice. They figured it out. So can your software.

Finally, I understand that this is a complex piece of software and that it takes effort to maintain and write. You undoubtedly have many other users who have other needs and desires. So thanks for listening and I hope you are able to fix this.

[lukastaegert]

There is no way that mangling an absolute path is something that was put there by design. That's crazy. Why would you want that? It's a side-effect of an algorithm designed to fix something else. It's a bug.

No it is not. This is the intended way of marking a file in an existing graph as "external", which is conflicting with your intentions:

// in rollup config
external: path.resolve('src/my-external-file.js'); // we provide the absolute path of the file that becomes external

This normalization makes sure that any import of that file knows it needs to be external. As plugins that resolve import locations do so by providing us with absolute ids, we need to be able to use an absolute id here as well.

As Rollup was traditionally more used for library bundling, this use case was much more common than the other use case of referencing a file on the server root.

If you have good suggestions on how to extend the API in a non-breaking way, this would be highly welcome and could receive adoption. Beyond that, your opinions are your own.

[arackaf]

I just got burned by this, too. Have to agree with @mzinbergs - I would expect resources marked as external to be left alone, for the executing context to handle.

That said, my use case was slightly more subtle. I got burned by writing my files to ./dist/. The mangled paths became invalid when run from this location! I thought maybe specifying a dir in the output config, and then a raw file was what rollup wanted, but it seems not.

I worked around this by just putting my output files in a place where the mangled import paths would just happily work, but this is far from ideal.

[lukastaegert]

If you have good suggestions on how to extend the API in a non-breaking way, this would be highly welcome and could receive adoption.

[arackaf]

@lukastaegert I guess if Rollup is insistent on adjusting paths to external resources, at the very least, adjusting this path so that it's correct if you write your output file to a different path on disk.

ie

output: {
    file: "./dist/result.js"),
    format: "esm",
  },

The paths I used will now need to adjust to the fact that they're loaded from ./dist, and so will likely need a ../ prepended. I hope that's clear. I assume it's a known issue?

[lukastaegert]

Well, it is a "known issue" in so far as it was designed that way. And once I accidentally changed that years ago there were immediately issues from people who relied on that, so I changed it back.

[arackaf]

@lukastaegert why was it designed like that? If these external imports are going to be adjusted, why wouldn't they be adjusted based on where the file is written to, and presumably executed from?

[guybedford]

This has always been a tricky API - I agree we should make a major break on this and implement a single sensible semantic for it.

[guybedford]

(what that is, I'm not quite sure, but just agreeing there is work to do here to improve)

[lukastaegert]

We could fix this non-breaking by adding a new output option, e.g. renormalizeExternalPaths. true would be the default and current behaviour, false would mean relative and absolute external paths just remain untouched (resulting in the potential issue where if files from different directories access the "same" external file, it will be two different files in the output), and a new semantic triggered by some string value, e.g. relative meaning that while relative paths will be deduplicated as before, absolute paths will remain untouched. I think that is what you are looking for, isn't it? And in the next major, we make relative the default.
This would also be a good point in the documentation where to describe the current behaviour. PR welcome.

[arackaf]

@lukastaegert yeah I think the behavior you describe would be ideal.

[guybedford]

Of course, externals that are bare specifiers should be left untouched by any renormalization.

Another useful change to consider alongside this work might be having RollupJS operate on URLs like Deno and Node.js now do instead of paths. The benefit of this is easily treating external URLs and local file URLs with the same semantics.

Formalizing the pathing system as URL based would also lead to other simplifications. Allowing opting out of URL behaviours by setting properties like this to false can still allow custom scheming.

[sutter-dave]

Just to add my two cents in case it is helpful. The origin of the problem for me is that when I use an absolute file reference for an import statement in my code, it is relative to a web server root, not the file system root. The code is client code that is accessed through my dev web server. When Rollup converts the absolute URL to a relative one, it appears to assume the absolute URL is taken relative to the file system root.

Possibly a way to fix this is to allow for the user to set the root directory for absolute paths. I think this would still allow the de-duping to work.

Of course doing this alone would only change the relative URL that is given at the end. I would want some way to keep it an absolute URL, preferably the same one I had specified.

[lukastaegert]

fix at #4021, please have a look.

[sutter-dave]

Thank you for addressing this. 👍 I read through the pull request. If I understand it selects an assumed root folder for the absolute references: "it assumes that the root of the generated bundle is located at the common shared parent directory of all modules that were included in the bundle" This is exactly the right answer for the way I organize my repos. My only comment is that I am not sure how general this is. In any case, I think it is a great addition. I appreciate the work you guys are doing. Dave

…

On Sat, Mar 27, 2021 at 6:22 AM Lukas Taegert-Atkinson < ***@***.***> wrote: fix at #4021 <#4021>, please have a look. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#3024 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAYYSY4VHRXQE5ZGZAZ5EB3TFXL2HANCNFSM4IJHCOYQ> .