Skip to content
This repository was archived by the owner on May 2, 2022. It is now read-only.

Conversation

@ehmicky
Copy link
Contributor

@ehmicky ehmicky commented Sep 24, 2020

Fixes #49.

This appends .js to the bundle filename.

On one hand, the bundle is merely an implementation detail, so we might want to keep it opaque.
On the other hand, many tools depends on knowing a file format using the file extension, so this might be a good practice. For example, when debugging a bundle, this would allow syntax highlighting in text editors and IDEs.

@ehmicky ehmicky self-assigned this Sep 24, 2020
@ehmicky ehmicky added the type: feature code contributing to the implementation of a feature and/or user facing functionality label Sep 24, 2020
@calavera
Copy link
Contributor

I don't think it matters, but can you test that the runtime still works with this change?

Copy link
Contributor

@calavera calavera left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I reviewed this changes with the manifest loader, and they should work as expected. My only comment is that the value in the SHA field is not a sha anymore.

/** @type {import("./upload").BundleInfo} */
const bundleInfo = {
sha: shasum.digest("hex"),
sha: `${shasum.digest("hex")}.js`,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we change this to not be named sha, since it has an extension?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can change it. How would you like it to be named?
I am assuming this would require changes in both Netlify Dev and the manifest loader though?

@ehmicky
Copy link
Contributor Author

ehmicky commented Sep 25, 2020

Since renaming sha would require additional work in two other projects, I'd like to make sure: is this something that we want? What do you think @calavera @mraerino @shortdiv?

@mraerino
Copy link
Contributor

mraerino commented Sep 25, 2020

can we put it on hold for now?

i think the sha field should stay as the raw sha and only the file itself should be renamed, but right now this breaks some stuff in the way we test traffic-mesh

@ehmicky
Copy link
Contributor Author

ehmicky commented Sep 25, 2020

Sounds good. It looks like this would be create more problems than it would solve, so I am closing this for now 👍

@ehmicky ehmicky closed this Sep 25, 2020
@ehmicky ehmicky deleted the feat/bundle-extension branch September 25, 2020 13:06
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

type: feature code contributing to the implementation of a feature and/or user facing functionality

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add a file extension to bundles

4 participants