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

automating winjs api genration #32

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Conversation

staxmanade
Copy link
Contributor

⚠️ this is currently not working...
I got it working. There was a strange caching problem with tscore.js processing multiple files.

But putting it up here to show my thinking... Any feedback would be great.

This work is coupled with the PR: winjs/winjs-control-apis#1

/cc @rigdern

This includes an updated grunt task that automatically updates
react-winjs.js with updated api metadata.
@rigdern
Copy link
Contributor

rigdern commented Oct 17, 2015

I appreciate the effort but I don't believe this is worth the work to automate. We have instructions on our wiki for maintainers describing how to update react-winjs for a new version of WinJS: https://github.com/winjs/react-winjs/wiki/Upgrading-react-winjs-to-a-new-version-of-WinJS

I think it would be sufficient to update that wiki page to give an example command to run. Something like:

"Assuming you are in a directory which has winjs-control-apis, winjs, and react-winjs as siblings, you can run this command to generate RawControlApis:"

> node winjs-control-apis/main.js winjs/typings/winjs.d.ts winjs/typings/mediaplayer.d.ts winjs/typings/tv.d.ts

For fun, if we were to automate this (again, I don't think it's worth the effort), here are my thoughts on the ideal solution.

I don't think we should check winjs.d.ts into react-winjs. Instead, when you want to update RawControlApis, the automation would download winjs.d.ts (and other needed d.ts files) from the WinJS repo from GitHub for the branch the developer is interested in.

Instead of replacing the definition of RawControlApis inside of react-winjs.js, we would move RawControlApis into its own file and the automation would recreate the file from scratch during the update. react-winjs.js would then require this RawControlApis.js file. That way we don't need to search for any special delimiters like //*** RawControlApis end.

@staxmanade
Copy link
Contributor Author

@rigdern - ok I think this is starting to look better (from a tooling perspective).

TODO

  • Update Examples with added/updated api surface
  • check-bom File is missing BOM: dist/npm/react-winjs.js is being reported with a grunt or grunt update-controls-api

@staxmanade
Copy link
Contributor Author

@rigdern

As a noob to the project - discovering how to get this going was rather difficult. So as much automation here as possible would be ideal.

I agree with both points you made - I tried to require(..) the api file in an earlier approach, but given the different build outputs it's not something I wanted to put time into yet. (Possibly browserifying during build could help?)

I saw this more as a stepping in the automation direction. How it ends up is worth iterating on.

@winjs winjs deleted a comment from msftclas Sep 27, 2017
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.

4 participants