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

Incorrect asset path when namer puts file in subdirectory. #4172

Open
kevincox opened this issue Feb 19, 2020 · 5 comments
Open

Incorrect asset path when namer puts file in subdirectory. #4172

kevincox opened this issue Feb 19, 2020 · 5 comments

Comments

@kevincox
Copy link
Contributor

kevincox commented Feb 19, 2020

🐛 bug report

When a namer puts different files in different directories import paths between the files are incorrect.

🎛 Configuration (.babelrc, package.json, cli command)

See the attached setup with a trivial namer and some basic files.

parcel-namer-bug.zip

% cd parcel-namer-bug
% npm ci
% npx parcel build index.html
✨ Built in 1.72s

dist/a/HASH_REF_2b6dd3ea98def065b1b5eba80b3132b6.js     130 B      0ms
├── node_modules/@parcel/runtime-js/lib/JSRuntime.js    158 B     22ms
├── index.html                                           66 B    733ms
└── index.js                                             64 B     32ms

dist/b5453a72.html                                       55 B    384ms
└── index.html                                           92 B    733ms

dist/a/4e3b3082.js                                       20 B    105ms
└── lib.js                                              191 B     27ms
% tree dist
dist
├── a
│   ├── 4e3b3082.js
│   └── 4e3b3082.js.map
└── b5453a72.html

1 directory, 3 files
% cat dist/*.html
<script type="module">import("./4e3b3082.js");</script>%  

🤔 Expected Behavior

The import statement should reference a file that exists. (./a/4e3b3082.js)

😯 Current Behavior

The import statement references a file that doesn't exist.

🔦 Context

It is unclear where this reference is being generated relative to. Because if you change the namer to put entires into a subdirectory and other files in the root it generates the exact same path for the import. (the proper path would be ../{hash}.js)

💻 Code Sample

Namer code (also in zip):

module.exports = new Namer({
	async name({bundle, bundleGraph, options}) {
		if (bundle.isEntry) {
			return `a/${bundle.hashReference}.${bundle.type}`;
		}
		return `${bundle.hashReference}.${bundle.type}`;
	}
});

🌍 Your Environment

Software Version(s)
Parcel 2.0.0-alpha.3.2
Node v13.8.0
npm 6.13.6
Operating System Linux
@mischnic
Copy link
Member

Your setup in the zip:

<script type="module">
	import "./index.js";
</script>
// index.js
import("./lib.js")

// lib.js
export default "foo";

The HTML asset is transformed and a child asset is generated for the inline JS asset (which gets a path like a/HASH1.js according to your namer), the async bundle is also emitted in a/HASH2.js.
So a JS asset in a/HASH1.js imports a JS asset in a/HASH2.js, so the relative path is just HASH2.js.
But when the inline asset is once again merged into the HTML asset, the "actual" path changes and so the import is wrong.

I think inline assets shouldn't get named by a Namer but only be assigned a name like (in this case) ${path.basename(html)}/JS-HASH.js. @devongovett @kwelch ?

@kevincox
Copy link
Contributor Author

That makes sense. There is no point in namers naming bundles that are never written somewhere. If they need to have a temporary name for processing then you proposal of some function of the name of the parent asset seems good.

@padmaia
Copy link
Contributor

padmaia commented Feb 20, 2020

Related: #4188. I like the idea of inline bundles not being named by namers. @mischnic, you suggested ${path.basename(html)}/JS-HASH.js. Is it important that inline bundle names include a hash, or could it be a stable id?

@mischnic
Copy link
Member

mischnic commented Feb 20, 2020

I think it doesn't have to be a real hash.
Maybe we could also just use the parent filepath (like the JSRuntime does with __filename)?

@kevincox
Copy link
Contributor Author

Just confirming that bug is still present on 2.8.3.

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

No branches or pull requests

3 participants