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

resolveId rewrites local ids without an escape hatch #2867

Closed
jescalan opened this issue May 20, 2019 · 10 comments · Fixed by #2907
Closed

resolveId rewrites local ids without an escape hatch #2867

jescalan opened this issue May 20, 2019 · 10 comments · Fixed by #2907

Comments

@jescalan
Copy link

I am trying to make a plugin that rewrites a specific external import to point to a specific path. For example, if I have

import foo from 'bar'

I'd like to modify it to be

import foo from './bar.json'

...for example. I have written a plugin that does this. As context, I have an external function that looks for the same id and ensures that the module is marked as not-external (since plugins do not run for external modules), then I catch the id I'm after and return a separate id, marked as external again. Here's roughly how it might look:

{
  resolveId: (id) => {
    if (id === 'bar') return { id: './bar.json', external: true }
  }
}

Expected Behavior / Situation

When I return an id from a plugin, that id will be what's seen in the final result. So I would expect to see import foo from './bar.json' in the output file.

Actual Behavior / Situation

After the plugin runs, some sort of logic rewrites the path to something different than I returned in the plugin. In my case, it looks like its trying to rewrite it such that it is relative to the source file's destination, but does so incorrectly, so I get back ./lib/bar.json.

I know this entire thing seems silly, but this is part of a plugin where I generate a file using other means into my dist folder, and so I just need to to respect the path that I give it, rather than trying to rewrite the path, or else it rewrites it incorrectly.

Modification Proposal

resolveId should respect the path you give to it. If you need it to resolve relative to the source file, you can always use the importee argument and manually resolve it this way. But honestly I don't care how it's resolved as long as there is some way that I can transform import foo from 'bar' to import foo from './asdjhaskdh' without my path being rewritten.

@jescalan jescalan changed the title resolveId rewrites ids without an escape hatch resolveId rewrites local ids without an escape hatch May 20, 2019
@lukastaegert
Copy link
Member

lukastaegert commented May 26, 2019

I agree that the resolution algorithm should probably be documented better. I would not actually change it as there are things like rollup-plugin-rebase that depend on Rollup's behaviour. But let me explain what is going on:

When a relative (i.e. starting with a .) import is marked as external, Rollup does the following:

  1. The relative import is resolved to an absolute path in your file system first. That way, if you have different modules in different folders that all import the same module and the imported module is made "external", the imports would still be resolved to the same external id (this is the reason why weird things are going on).
  2. The absolute path is then resolved to a path relative to the common base directory of all entry modules. That is, if your only entry point module is src/lib1/main.js, the common base directory is src/lib1 while if you add a second entry point src/lib2/main.js it will be src. Therefore if your entry points import src/lib3/external.js via a relative import, then this relative path is ../lib3/external.js if there is only one entry point and ./lib3/external.js if there are two.
  3. In the generated chunks, use this relative path for the external import, possibly adjusted if the chunk itself is in a nested folder relative to dir.

@lukastaegert
Copy link
Member

The important part here is that this logic allows marking individual modules in a module graph as external while making sure all imports of this module still point to the same external id.

@lukastaegert
Copy link
Member

One approach for your plugin could be to not return a relative id but an absolute id. The only tricky part is figuring out the common base directory. Considering the different ways Rollup's input can be structured, this should work:

import path from "path";
import commonDir from "common-dir";

export default () => {
  let baseDir;
  return ({
    options({input}) {
      const entryModules =
        typeof input === 'string'
          ? [input]
          : Array.isArray(input)
          ? input
          : Object.keys(input).map(name => input[name]);
      baseDir = commonDir(
        entryModules.map(relativePath => path.resolve(relativePath))
      );
    },
    resolveId(id) {
      if (id === 'bar')
        return {
          id: path.resolve(baseDir, 'bar.json'),
          external: true
        };
    }
  });
};

@lukastaegert
Copy link
Member

Nevertheless I see this is a lot of effort. As the object form of resolveId is relatively new, we could consider switching the logic for relative external ids when using the object form to never change the returned id, and this could be your feature request. Not sure if this would require a major release as depending on the point of view, this could be anything from a bug fix to a breaking change.

@jescalan
Copy link
Author

jescalan commented Jun 6, 2019

The code you posted here is working for me - thank you! It does seem a bit involved though for what feels like it should be a simple use case

@lukastaegert
Copy link
Member

PR at #2907

@jescalan
Copy link
Author

Woah, thank you @lukastaegert - this is great! So once this is out, the original code in the issue above would work as expected, or are there some changes I should make?

@lukastaegert
Copy link
Member

Yes it should.

@lukastaegert
Copy link
Member

And it is already released

@jescalan
Copy link
Author

Amazing, thank you! Just removed the workaround from our build code and it is so much better. Incredibly strong OSS maintenance here 🙏

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

Successfully merging a pull request may close this issue.

2 participants