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

RFC: Remove build output from repository #765

Closed
phillipj opened this issue Nov 2, 2020 · 7 comments · Fixed by #772
Closed

RFC: Remove build output from repository #765

phillipj opened this issue Nov 2, 2020 · 7 comments · Fixed by #772

Comments

@phillipj
Copy link
Collaborator

phillipj commented Nov 2, 2020

Intro

We want to remove the build output (mustache.js and mustache.min.js) from the git repository.

This has been kept around for historical reasons. Being allowed to use github.com directly in a <script> element is really convenient and sufficient for many. Until recent years, there has not been viable approaches for using <script> elements with released versions of mustache.js available on npmjs.com.

This request-for-comments issue is created to give consumers a heads-up and open up for them to raise their concerns.

Currently the plan is to remove the build output from the git repository in the start of December 2020.

Why?

There are numerous reasons having the build output in the git repository causes pain, most importantly:

  1. Consuming developers gets code running that has not been in an official release yet
  2. Pull requests gets noisy and confusing as there are changes in both the source code (mustache.mjs) and the build output

Who will be affected?

Those downloading mustache.js directly from github.com in their website:

<script src="https://raw.githubusercontent.com/janl/mustache.js/master/mustache.min.js"></script>

What's the alternative?

Download mustache.js from unpkg.com instead:

<script src="https://unpkg.com/mustache@latest"></script>

(ideally with a version specifier instead of latest, more on that on unpkg.com)

Will this affect the npm package?

No.

@phillipj
Copy link
Collaborator Author

phillipj commented Dec 25, 2020

A braindump as a future reference before starting on the work to make this happen:

The mustache.js file has been the source file for this package since the beginning. Deleting that file from this git repository would be a shame since it would make following the history of changes more challenging and our great contributors throughout time wouldn't get their well deserved credit.

These days mustache.mjs is the source file.

With the above in mind, ensuring we keep backwards compatibility & mustache.js's history in tact, I'm aiming for:

In the git repository:

  • mustache.js for the source file, but it to be written in ES modules syntax like mustache.mjs is today
  • Delete mustache.mjs
  • No other build output files to exist

In the npm package:

  • mustache.js to be the build output with an UMD wrapper like today
  • mustache.min.js minified version of mustache.js
  • mustache.mjs to have ES modules syntax

@phillipj
Copy link
Collaborator Author

phillipj commented Jan 7, 2021

I've been playing around with this in the remove-build-output-from-git branch lately.

Most challenges faced so far has been fixable, but I just realised removing mustache.mjs from the git repository might cause havoc for deno projects.. deno projects often imports modules directly from github.com. Hence my initial plan of only having mustache.mjs in the npm package, isn't without downsides 🤔

@phillipj
Copy link
Collaborator Author

phillipj commented Jan 7, 2021

@zekth you were of big help about a year ago when the source code transition to being written in ESM in mustache.mjs (#728), primarily to support deno.

Are you still into using deno these days? I'm really curious if you know of any protips that would allow us to remove mustache.mjs from this git repository, without causing too much pain for deno users?

If there's no way for us to make that transition completely transparent for deno users, maybe at least introduce a deprecation warning only trigger when executed via deno a while before actually deleting mustache.mjs.

@zekth
Copy link
Contributor

zekth commented Jan 8, 2021

Lately i'm not on Deno but still following the development. There is several package registries and @kitsonk worked on making pika.dev work properly with Deno. It now has moved to https://www.skypack.dev/ and seems like the more reliable way to move forward for mustache.

@phillipj
Copy link
Collaborator Author

phillipj commented Jan 8, 2021

Thanks for the quick reply!

Interesting. I for some reason had the impression deno users had a strong github.com preference. Valuable to know that's not correct and skypack.dev, unpkg.com etc is on the table.

That obviously makes a big difference as we can recommend end-users using the build output wrapped into the npm package, consumed from whatever service of their preference, rather than the source code on github.com directly.

Still would be a hard one-time breaking change for current deno users tho.

@kitsonk
Copy link

kitsonk commented Jan 8, 2021

For stuff built on Deno, people throw it up on GitHub and register the package at https://deno.land/x/ or https://nest.land/. skypack.dev will generally grab anything on npm and focuses on ES Modules (which Deno loves) but also provides the ability to set the TypeScript types header if the package query includes ?dts, which Deno will go and fetch and allow JavaScript ES Modules to be strongly typed checked. I believe skypack.dev will look at the package.json for the types if they are included in the package, or try to determine them via the @types namespace on npm.

Personal opinion is that if it isn't built for Deno, focusing on maximising compatibility with skypack.dev should work for Deno.

@phillipj
Copy link
Collaborator Author

phillipj commented Jan 8, 2021

Much appreciated @kitsonk!

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

Successfully merging a pull request may close this issue.

3 participants