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

Enable absolute imports and minify distributable #2132

Merged
merged 24 commits into from
May 31, 2019
Merged

Conversation

Tyriar
Copy link
Member

@Tyriar Tyriar commented May 26, 2019

Fixes #1314
Fixes #1254


This PR seems like a good place to start to move to v4 as it solves a bunch of problems and will let us remove the browserify build step as it works in lib/ now. If you want to review alt+click the collapse button in GitHub to collapse all files and then ignore all files inside src/ except for tsconfig.json.

It makes the following high-level changes:

  • Enables absolute imports via baseUrl and paths in tsconfig.json for all paths referencing common, core and ui modules:
    // before
    import { DEFAULT_COLOR } from '../../common/Types';
    import { IColorSet } from '../../ui/Types';
    // after
    import { DEFAULT_COLOR } from 'common/Types';
    import { IColorSet } from 'ui/Types';
  • Moved the tsc build output to out/, this is no longer suitable to directly import because the absolute paths need to be packaged.
  • Webpacks out/ as a production umd module in lib/, this is the new main file in `package.json
  • The demo imports from out/ by default but can import from lib/ by just uncommenting a line to verify the packaged portion (it requires you to run yarn package though):
    // Use tsc version (yarn watch)
    import { Terminal } from '../out/public/Terminal';
    // Use webpacked version (yarn package)
    // import { Terminal } from '../lib/xterm';
  • Removed gulp and lots of dependencies
  • Replaced gulp tests with npm scripts, removed build gulp tasks completely
  • Removed coverage reporting as it wasn't helping and can be misleading
  • Moved xterm.css into css/, removed tasks for move it around

Follow up changes after this look good:

  • Sort imports
  • Remove src/addons and all build steps completely
  • Update documentation:
    • dist/ -> lib/
    • New css path

Notes:

  • While ts-loader is capable of doing the build in a single step, we lose great tsc features like strict null checking and composite projects. Splitting it up like it has always been also makes it much easier to understand imo.

@Tyriar Tyriar added this to the 4.0.0 milestone May 26, 2019
@Tyriar Tyriar requested a review from a team May 26, 2019 19:16
@Tyriar Tyriar self-assigned this May 26, 2019
@Tyriar
Copy link
Member Author

Tyriar commented May 26, 2019

Need to figure out how to resolve the paths for the tests.

@Tyriar Tyriar added the breaking-change Breaks API and requires a major version bump label May 26, 2019
@Tyriar
Copy link
Member Author

Tyriar commented May 31, 2019

And so it begins.

@Tyriar Tyriar merged commit fad6e0f into xtermjs:master May 31, 2019
@Tyriar Tyriar deleted the ts_wp branch May 31, 2019 00:01
Tyriar added a commit to Tyriar/xterm.js that referenced this pull request May 31, 2019
No longer needed after webpack refactor

Missed in xtermjs#2132
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking-change Breaks API and requires a major version bump
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Use tsconfig baseUrl and absolute imports Is there a need for a build/ and a dist/ folder?
1 participant