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

no 'dist' build in releases ? #1201

Closed
flumm opened this issue Jan 8, 2018 · 23 comments
Closed

no 'dist' build in releases ? #1201

flumm opened this issue Jan 8, 2018 · 23 comments
Assignees

Comments

@flumm
Copy link
Contributor

flumm commented Jan 8, 2018

up to 2.9.2 the release tags included a 'dist' directory with the built package, ready for consumption

since 3.0.0 there is no 'dist' directory anymore, is this by design or accident?

self building would be very hard for us (proxmox) since on debian (which we use as base) neither npm nor gulp is packaged, and it would mean we would rely on non-debian repos for this (or a not very nice build-workflow)

edit:

afaics, the dist/ is in the .gitignore (from the v3 branch) with a comment that this can be removed when merging v3 into master, but this did not happen, so i hope this was just a mistake

@Tyriar
Copy link
Member

Tyriar commented Jan 8, 2018

It's probably a good idea to keep shipping this, but we should still keep it in the .gitignore. Thoughts @parisk?

@FGasper
Copy link
Contributor

FGasper commented Jan 8, 2018

In the meantime, is there a way to build the dist/ directory as before? build/ doesn’t include AMD loaders, which we use extensively in our application.

@Tyriar
Copy link
Member

Tyriar commented Jan 9, 2018

Looks like the build task might have been removed to build dist. To use xterm.js do you install it from npm and then link to the files in node_modules/xterm/dist/ from html?

@FGasper
Copy link
Contributor

FGasper commented Jan 9, 2018

Previously we just packaged up dist/ into an RPM.

@blink1073
Copy link
Contributor

blink1073 commented Jan 9, 2018

FWIW @cancan101 has found a way for us to continue pulling in xterm using bower in Jupyter Notebook: jupyter/notebook#3189

@flumm
Copy link
Contributor Author

flumm commented Jan 9, 2018

@Tyriar we use the git as a submodule for our own package and use the dist directory direclty in our build
as we cannot really use npm (see my comment about debian)

@parisk
Copy link
Contributor

parisk commented Jan 9, 2018

OK, this was not by any means intentional 😅 . Will take a look.

@Tyriar
Copy link
Member

Tyriar commented Jan 9, 2018

@parisk I was looking at the old issues as the decisions are a little hazzy for me and the plan was to continue shipping dist/ after removing bower support. Just got lost in the noise of the webpack build I guess.

@parisk
Copy link
Contributor

parisk commented Jan 10, 2018

@Tyriar you are right. I am trying to understand why this happened at all tbh. Seems like dist files are supposed to be included in the released, based on package.json on release/3.0:

xterm.js/package.json

Lines 19 to 22 in 45ed27d

"dist/*.js",
"dist/*.js.map",
"dist/**/*.js",
"dist/**/*.js.map",

@blink1073
Copy link
Contributor

Perhaps the issue is the git release not containing the dist files, as in they used to be checked in to the repo?

@blink1073
Copy link
Contributor

@parisk from my local test the dist files are included when installing from npm.

@flumm
Copy link
Contributor Author

flumm commented Jan 10, 2018

@blink1073 @parisk @Tyriar just for clarity: i do not talk about npm releases (as i already explained we cannot use npm) but about the release tags in git (as we use this for our packaging)

afaics, during the development of the v3 branch, the dist directory was included in the .gitignore, so that
building there does not pollute the git repository with a comment that this can be removed when merging. a possible solution would be to manually add those files in the release script and leave it in the gitignore

edit: after reading #406 , it seems that this was the plan all along, so sorry for the confusion, but is there really no other way to get the released dist files other than npm? the issue talks about build artifacts, but it seems it is not available this way either.

@parisk
Copy link
Contributor

parisk commented Jan 10, 2018

Yep, I was trying to say that right now 😅 , thanks @blink1073! The dist directory is being included in the npm release appropriately.

@flumm the suggested way to install xterm.js is via npm or yarn: https://xtermjs.org/docs/guides/download/.

Shipping the dist directory in the Git repository has turned out to be cumbersome and error prones (accidental commits etc), so we stopped doing so.

If you insist installing xterm via Git, I suggest you write a wrapper script to also perform the build for you, after pulling the latest changes.

@parisk parisk closed this as completed Jan 10, 2018
@flumm
Copy link
Contributor Author

flumm commented Jan 10, 2018

@parisk is it really not possible to include the dist as a build artifact on git like you described in #406 ? if not, that means that for the foreseeable future we will not be able to update to a newer xtermjs release than 2.9.2 since neither npm nor yarn is available on debian 9 (adding external repositories in the build step is no really feasible for us).

@blink1073
Copy link
Contributor

blink1073 commented Jan 10, 2018

@flumm, why not pull the package from npmjs.org instead of github?

$ npm view xterm dist.tarball
https://registry.npmjs.org/xterm/-/xterm-3.0.1.tgz

@parisk
Copy link
Contributor

parisk commented Jan 10, 2018

@flumm no, we do not have any intentions to maintain distribution files in the repository, as we believe it's a bad practice.

You can find more information about this decision on the initial issue and PR:

@flumm
Copy link
Contributor Author

flumm commented Jan 10, 2018

@blink1073 yes that seems to work :) thanks

@parisk yeah, in general i agree with you, but sadly with npm in combination with debian it is not easy to integrate stuff where the you can only build with npm/yarn/etc.
i simply missed the whole issue because i integrated xterm.js in proxmox well after those decisions were made

@parisk
Copy link
Contributor

parisk commented Jan 10, 2018

@flumm I see your use case. You can give a try at Docker for your needs (if it's applicable to you). You can use the node image to build your project.

Or you can just use the npm tarballs 😄.

All the best!

@cancan101
Copy link

What about just using nvm to install npm?

@Greenek
Copy link

Greenek commented Jan 13, 2018

Feel free to use bower install --save xterm.js-next. It's my fork created for personal use, but I try to keep it updated.

@fake-name
Copy link

Wait, so you have to have npm and/or yarn installed just to get 2 files?

This is spectacularly ridiculous. I'm working on a project that doesn't use node at all, so I don't have any of the tooling. It does have a clientside web component, so I'd like to use xtermjs. However, apparently you can't actually get xtermjs.

People who don't use node exist. Requiring it just to get the distribution js files for serving is insane.

@raquelhortab
Copy link

raquelhortab commented May 10, 2021

I completely agree with @fake-name. It is quite annoying having to clone and build the project just to get the distribution .js and .css. I've already cloned and built the project but still cannot find the files, should there be a dist or build folder somewhere? Can anyone explain how should I generate those files?

@Tyriar
Copy link
Member

Tyriar commented May 10, 2021

@raquelhortab I think you're talking about something different, @fake-name wants the files without using node, which is available for example on https://www.jsdelivr.com/package/npm/xterm. I'm not entirely sure what you're after, perhaps you're missing running yarn package which generates the lib folder that is pointed to from main in package.json.

Regardless, both of these topics are unrelated to this original issue, feel free to create a new one to discuss but going to lock this as it's old and already resolved.

@xtermjs xtermjs locked as off-topic and limited conversation to collaborators May 10, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

9 participants