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

feat: create ES5 build by using tsc on build directory #6249

Closed
wants to merge 3 commits into from

Conversation

wylieconlon
Copy link

Following up on previous attempts to create an ES5 build of vega-lite (#5920 and #5619), I have seen @domoritz request a simpler build process. The simplest process I was able to come up with is to use the Typescript compiler on the Javascript build, with an ES5 target.

This PR adds a step yarn build:es5, which takes the build/vega-lite.js file as input, and outputs the directory:

build-es5/vega-lite.js
build-es5/vega-lite.d.ts
build-es5/vega-lite.d.ts.map
build-es5/tsconfig.build-es5.tsbuildinfo

There is also a step added to yarn postbuild which adds:

build-es5/vega-lite.min.js

I verified this change in two ways:

  • I used an AST walker to verify that there are no ES6 features used in the built file.
  • I tested the built version of vega-lite in IE11 in my virtual machine, and was able to use the latest vega features such as color palettes from 5.0

Please:

  • Make the pull requests (PRs) atomic. (Fix one issue at a time.) Multiple relevant issues that must be fixed together? Make atomic commits so we can easily review each issue.
  • Provide a concise title so we can easily copy it to the release note.
    • Use imperative mood and present tense.
    • Mention relevant issues. (e.g., Fixes #1 / Fixes part of #1)
  • Lint and test (Run yarn test)
  • Rebase onto the latest master branch.
  • Review your changes before sending the PR (to ensure code quality).
  • For new features:
    • [ ] Add new unit tests.
    • [ ] Update the documentation under site/docs/ + add examples.

@wylieconlon wylieconlon mentioned this pull request Apr 3, 2020
7 tasks
@domoritz
Copy link
Member

domoritz commented Apr 3, 2020

Thank you! Would it be simpler to have a compile flow similar to es6? We could add an tsconfig for es5 in src. having the same flow for es5 and es6 would make it easier to debug issues, I think. What do you think?

@wylieconlon
Copy link
Author

@domoritz I did try the approach that I think you're describing, which was to put a second tsconfig file in the src directory, inheriting from tsconfig.src.json and setting target: ES5. That approach did not work 100%, because there were 5 uses of ES6 syntax in the built file. I believe the ES6 syntax came from the polyfills that vega-lite includes as dependencies, and prevented its use in IE11.

@domoritz
Copy link
Member

domoritz commented Apr 3, 2020

I see. So the issue is that things imported by Vega-Lite are not necessarily es5. Maybe using babel is the right choice then. I have to think about this a bit more. Thanks for showing that this approach works.

@domoritz
Copy link
Member

domoritz commented Apr 6, 2020

Fixed with babel in #6280. I think eventually we should switch to babel for all our builds but that means we need to remove namespaces #5913.

Thank you for the pull requests.

@domoritz domoritz closed this Apr 6, 2020
@wylieconlon wylieconlon deleted the es5-build branch April 6, 2020 03:26
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.

None yet

2 participants