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

onResolve shouldn't change output path for entry points #945

Closed
lahmatiy opened this issue Mar 10, 2021 · 7 comments · Fixed by #1078
Closed

onResolve shouldn't change output path for entry points #945

lahmatiy opened this issue Mar 10, 2021 · 7 comments · Fixed by #1078
Labels

Comments

@lahmatiy
Copy link

lahmatiy commented Mar 10, 2021

By default, esbuild replicates file structure into the output directory. So, the following setup:

require('esbuild').build({
    entryPoints: [
        'out_test/bar/index.js',
        'out_test/foo/index.js'
    ],
    bundle: true,
    outdir: 'out_test_build'
});

Will produce a result on outdir:

out_test_build
   bar
      index.js
   foo
      index.js

That's what we expected. However, if we're using onResolve for an entry point and onLoad to generate a content this way:

require('esbuild').build({
    entryPoints: [
        'out_test/bar/index.js',
        'out_test/foo/index.js'
    ],
    bundle: true,
    outdir: 'out_test_build',
    plugins: [{
        name: 'example',
        setup({ onResolve, onLoad }) {
            onResolve({ filter: /.*/ }, args => {
                if (args.kind === 'entry-point') {
                    return {
                        namespace: 'test',
                        path: 'ref',
                        pluginData: args
                    };
                }
            });
            onLoad({ namespace: 'test', filter: /.*/}, args => {
                return {
                    contents: 'console.log("generated content")'
                };
            });
        }
    }]
});

We get an error:

 > error: Duplicate entry point "test:ref"

~/tmp/esbuild/node_modules/esbuild/lib/main.js:1195
  let error = new Error(`${text}${summary}`);
              ^

Error: Build failed with 1 error:
error: Duplicate entry point "test:ref"
    at failureErrorWithLog (~/tmp/esbuild/node_modules/esbuild/lib/main.js:1195:15)
    at buildResponseToResult (~/tmp/esbuild/node_modules/esbuild/lib/main.js:920:32)
    at ~/tmp/esbuild/node_modules/esbuild/lib/main.js:1019:20
    at ~/tmp/esbuild/node_modules/esbuild/lib/main.js:566:9
    at handleIncomingPacket (~/tmp/esbuild/node_modules/esbuild/lib/main.js:655:9)
    at Socket.readFromStdout (~/tmp/esbuild/node_modules/esbuild/lib/main.js:533:7)
    at Socket.emit (node:events:378:20)
    at addChunk (node:internal/streams/readable:313:12)
    at readableAddChunk (node:internal/streams/readable:288:9)
    at Socket.Readable.push (node:internal/streams/readable:227:10) {
  errors: [
    {
      detail: undefined,
      location: null,
      notes: [],
      text: 'Duplicate entry point "test:ref"'
    }
  ],
  warnings: []
}

It is surprising that we can't resolve entry points to the same reference, since later its content may be loaded/generated (using onLoad) depending on other resolve parameters. But ok, let's keep original path and pass extra parameters through pluginData:

require('esbuild').build({
    entryPoints: [
        'out_test/bar/index.js',
        'out_test/foo/index.js'
    ],
    bundle: true,
    outdir: 'out_test_build',
    plugins: [{
        name: 'example',
        setup({ onResolve, onLoad }) {
            onResolve({ filter: /.*/ }, args => {
                if (args.kind === 'entry-point') {
                    return {
                        namespace: 'test',
                        path: args.path,
                        pluginData: {
                            ref: 'something'
                        }
                    };
                }
            });
            onLoad({ namespace: 'test', filter: /.*/}, args => {
                return {
                    contents: 'console.log("generated content")'
                };
            });
        }
    }]
});

In this case, we get another error:

 > error: Two output files share the same path but have different contents: out_test_build/index.js

~/tmp/esbuild/node_modules/esbuild/lib/main.js:1195
  let error = new Error(`${text}${summary}`);
              ^

Error: Build failed with 1 error:
error: Two output files share the same path but have different contents: out_test_build/index.js
    at failureErrorWithLog (~/tmp/esbuild/node_modules/esbuild/lib/main.js:1195:15)
    at buildResponseToResult (~/tmp/esbuild/node_modules/esbuild/lib/main.js:920:32)
    at ~/tmp/esbuild/node_modules/esbuild/lib/main.js:1019:20
    at ~/tmp/esbuild/node_modules/esbuild/lib/main.js:566:9
    at handleIncomingPacket (~/tmp/esbuild/node_modules/esbuild/lib/main.js:655:9)
    at Socket.readFromStdout (~/tmp/esbuild/node_modules/esbuild/lib/main.js:533:7)
    at Socket.emit (node:events:378:20)
    at addChunk (node:internal/streams/readable:313:12)
    at readableAddChunk (node:internal/streams/readable:288:9)
    at Socket.Readable.push (node:internal/streams/readable:227:10) {
  errors: [
    {
      detail: undefined,
      location: null,
      notes: [],
      text: 'Two output files share the same path but have different contents: out_test_build/index.js'
    }
  ],
  warnings: []
}

Looks like esbuild is losing dir path for entry points. I tried outbase but it has no effect.

Is this the expected behaviour?

I believe an entry point path should be untouched disregarding onResolve/onLoad is used or not. Otherwise it's lead to unexpected errors. Also make hard to recognise entry points in a result, since paths are different from those which we pass to entryPoints. As a workaround for now, we map entry point paths to an unique name (like 1.js, 2.css, 3.js etc) and re-map them back in result's outputFiles.

@zaydek
Copy link

zaydek commented Mar 10, 2021

If it’s not too much trouble to ask, can you provide a minimal reproducible repo?

@evanw
Copy link
Owner

evanw commented Mar 10, 2021

This happens because esbuild only interprets paths in the file namespace as file system paths. Paths in other namespaces are considered to be a custom path encoding scheme and are treated as strings of arbitrary data instead. They might be HTTP URLs for example, in which case it wouldn't make sense to try to compute the path to them relative to outbase. For paths not in the file namespace, esbuild tries to generate a reasonable-looking file name for the resulting output file. In this case the "reasonable-looking file name" algorithm is generating the same name and that causes a collision.

An easy solution to this is to just use the file namespace if these paths do indeed represent file system paths. A more advanced solution could involve a plugin API for setting the output path for individual output files (not implemented yet but tracked by #553).

@lahmatiy
Copy link
Author

@evanw Your conclusions are clear and I agree with them (I read some threads about it). However, my assumption is to find the same file paths as I passed to entryPoints. It's not clear why entry point output file path is changing based on onResolve result. So even with file namespace I get file path other than I pass in entryPoints, for instance:

const path = require('path');

require('esbuild').build({
    entryPoints: [
        'out_test/foo.js'
    ],
    bundle: true,
    outdir: 'out_test_build',
    plugins: [{
        name: 'example',
        setup({ onResolve }) {
            onResolve({ filter: /.*/ }, args => {
                if (args.kind === 'entry-point') {
                    return {
                        path: path.resolve('out_test/bar.js')
                    };
                }
            });
        }
    }]
});

In this case I've got bar.js in output (or in result.outputFiles with write: false), rather than foo.js as I expected. So, currently I need to rely on plugin's logic rather than on esbuild setup.

In short, my concern is about keeping file path for entry points, since we are looking for them in output. If entry point file paths are changing somehow (according to internal rules), there are not clue how to match input entry point files and output. Even metafile has no any mention of foo.js (content of metafile for the example above):

{
    "inputs": {
        "out_test/bar.js": {
            "bytes": 23,
            "imports": []
        }
    },
    "outputs": {
        "out_test_build/bar.js": {
            "imports": [],
            "exports": [],
            "entryPoint": "out_test/bar.js",
            "inputs": {
                "out_test/bar.js": {
                    "bytesInOutput": 25
                }
            },
            "bytes": 61
        }
    }
}

I believe that onResolve/onLoad should affect internal mechanics only, not esbuild setup or output file paths (at least for entry points).

@lahmatiy
Copy link
Author

They might be HTTP URLs for example, in which case it wouldn't make sense to try to compute the path to them relative to outbase. For paths not in the file namespace, esbuild tries to generate a reasonable-looking file name for the resulting output file. In this case the "reasonable-looking file name" algorithm is generating the same name and that causes a collision.

Looks like, a case with a HTTP URLs doesn't work either:

onResolve({ filter: /.*/ }, args => {
    if (args.kind === 'entry-point') {
        return {
            namespace: 'test',
            path: 'http://example.com/generate?' + args.path
        };
    }
});
onLoad({ namespace: 'test', filter: /.*/}, args => {
    return {
        // actually there should be fetch() or something like that
        contents: 'console.log(' + JSON.stringify(args.path) + ')'
    };
});

This example also results in the error "Two output files share the same path but have different contents".

@lahmatiy
Copy link
Author

lahmatiy commented Mar 11, 2021

@evanw Caught myself thinking that maybe you meant that non-file reference might to be used in entryPoints. Something like that:

require('esbuild').build({
    entryPoints: [
        'unicorn',
        'http://example.com/whatever'
    ],
    ...
});

That makes sense. Although this looks very exotic. Anyway, it's still unclear how onResolve might be helpful for file path producing, since it might just to switch a namespace and all the magic will happen in onLoad. It seems that even in this case, the generation of output file paths should come from the values in entryPoints, rather than from onResolve results. Also, such references can't be loaded with no plugin, therefore there is no reason in such references without a plugin. And if we use a plugin, it's reasonable to use a reference in file namespace (since it might be used for output file path generation) and do all the magic in plugin's onResolve handler (i.e. map a file path to something else, e.g. http url or whatever). Anyway, there is no options to match input references and output file paths.

@evanw
Copy link
Owner

evanw commented Mar 12, 2021

Ah, I see. I think this sounds like a reasonable change. Unfortunately it's a breaking change so I'd like to wait for the next round of breaking changes to fix this.

Along with that change, I think it would also be good to optionally support an entry point map for further flexibility, although that's another breaking API change on the Go public API side:

require('esbuild').build({
    entryPoints: {
        'output-name-1.js': 'unicorn',
        'output-name-2.js': 'http://example.com/whatever'
    ],
    ...
});

@evanw evanw added the breaking label Mar 21, 2021
@matthewmueller
Copy link

matthewmueller commented Mar 27, 2021

At the risk of this being an elaborate +1, I'd like to share my use case. I think would be resolved by #553 or the entrypoint map.

A useful pattern for working with .svelte or .vue entrypoints is to wrap the compiled file in a JS envelope that attaches it to the DOM. Something like:

import View from "./index.svelte"

new View({
	props: JSON.parse(document.getElementById('props').textContent),
	target: document.getElementById('container'),
	hydrate: true
})

Ideally, this JS file is a virtual entrypoint and you just have index.svelte in your filesystem. Right now, if you have index.svelte and about/index.svelte, you'll get the error described above:

Two output files share the same path but have different contents: index.js

I'm not sure the best way to solve this. In this case, these are virtual files, but since they are file-like I'd expect the output path to be ./index.js and ./about/index.js, but I can see how other schemes like HTTP complicate the output path rules.

I should be able to workaround this issue by appending JS to the bottom of the compiled Svelte file and being careful about scope. No virtual files needed then.

One more thing is that this used to work after this issue. I'm picking this project back up so the code hasn't changed much since then 😊

Update: The workaround of appending JS to the bottom worked fine. It's cool to see how the variables get renamed but the names still match up because the renaming happens later in the pipeline. 🙂

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

Successfully merging a pull request may close this issue.

4 participants