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

Revert "SerializeAsHTML(): Fallback to default ansi colors when running headless terminal" #4333

Merged
merged 1 commit into from
Dec 19, 2022

Conversation

Tyriar
Copy link
Member

@Tyriar Tyriar commented Dec 19, 2022

Reverts #4196

This caused an issue when packaging v5.1:

assets by status 10.3 KiB [cached] 1 asset
./out/SerializeAddon.js 22.5 KiB [built] [code generated]

ERROR in ./out/SerializeAddon.js 4:23-63
Module not found: Error: Can't resolve 'browser/services/ThemeService' in '/home/vsts/work/1/s/addons/xterm-addon-serialize/out'
resolve 'browser/services/ThemeService' in '/home/vsts/work/1/s/addons/xterm-addon-serialize/out'
  Parsed request is a module
  using description file: /home/vsts/work/1/s/addons/xterm-addon-serialize/package.json (relative path: ./out)
    Field 'browser' doesn't contain a valid alias configuration
    resolve as module
      /home/vsts/work/1/s/addons/xterm-addon-serialize/out/node_modules doesn't exist or is not a directory
      /home/vsts/work/1/s/addons/xterm-addon-serialize/node_modules doesn't exist or is not a directory
      /home/vsts/work/1/s/addons/node_modules doesn't exist or is not a directory
      looking for modules in /home/vsts/work/1/s/node_modules
        /home/vsts/work/1/s/node_modules/browser doesn't exist
      /home/vsts/work/1/node_modules doesn't exist or is not a directory
      looking for modules in /home/vsts/work/node_modules
        /home/vsts/work/node_modules/browser doesn't exist
      /home/vsts/node_modules doesn't exist or is not a directory
      /home/node_modules doesn't exist or is not a directory
      /node_modules doesn't exist or is not a directory

webpack 5.61.0 compiled with 1 error in 747 ms
npm ERR! code ELIFECYCLE
npm ERR! errno 1
npm ERR! xterm-addon-serialize@0.9.0 package: `../../node_modules/.bin/webpack`
npm ERR! Exit status 1
npm ERR! 
npm ERR! Failed at the xterm-addon-serialize@0.9.0 package script.
npm ERR! This is probably not a problem with npm. There is likely additional logging output above.
npm WARN Local package.json exists, but node_modules missing, did you mean to install?

npm ERR! A complete log of this run can be found in:
npm ERR!     /home/vsts/.npm/_logs/2022-12-19T18_07_51_333Z-debug.log
npm ERR! code ELIFECYCLE
npm ERR! errno 1
npm ERR! xterm-addon-serialize@0.9.0 prepublishOnly: `npm run package`
npm ERR! Exit status 1
npm ERR! 
npm ERR! Failed at the xterm-addon-serialize@0.9.0 prepublishOnly script.
npm ERR! This is probably not a problem with npm. There is likely additional logging output above.
npm WARN Local package.json exists, but node_modules missing, did you mean to install?

npm ERR! A complete log of this run can be found in:
npm ERR!     /home/vsts/.npm/_logs/2022-12-19T18_07_51_363Z-debug.log
Spawn exited with code 1
##[error]Bash exited with code '1'.

@Tyriar Tyriar added this to the 5.1.0 milestone Dec 19, 2022
@Tyriar Tyriar self-assigned this Dec 19, 2022
@Tyriar Tyriar merged commit 19c760a into master Dec 19, 2022
@Tyriar Tyriar deleted the revert-4196-ansiheadless branch December 19, 2022 18:20
@silamon
Copy link
Contributor

silamon commented Dec 19, 2022

Any idea how to fix this? It's the import for DEFAULT_ANSI_COLORS that is making it fail. Duplicating the function to serialize addon seems not the right solution.

@Tyriar
Copy link
Member Author

Tyriar commented Dec 19, 2022

Something like this should do it:

https://github.com/Tyriar/xterm.js/blob/30ef1fcaa5b522e3d05ce000c63c7c5300970550/addons/xterm-addon-webgl/webpack.config.js#L24-L31

Almost made that change but I was trying to resolve the partial release asap. We should also try package as part of regular CI so we would catch this case in the future.

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 this pull request may close these issues.

2 participants