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

Flag Libraries with Advanced Compilation Issues #508

Closed
xcthulhu opened this issue Apr 10, 2016 · 7 comments
Closed

Flag Libraries with Advanced Compilation Issues #508

xcthulhu opened this issue Apr 10, 2016 · 7 comments

Comments

@xcthulhu
Copy link
Contributor

From my own experience and others, not all libraries on CLJSJS support advanced compilation.

In particular, my own BitAuth contribution does not. This appears to be due to its use of an outdated version of elliptic but I'm not sure).

I understand vega may also have issues.

It would be good to note which libraries have advanced compilation issues so folks can make informed decisions before building whole apps around them only to get bitten.

@xcthulhu xcthulhu changed the title FlagLibraries that Don't Support Advanced Compilation Flag Libraries with Advanced Compilation Issues Apr 10, 2016
@Deraen
Copy link
Member

Deraen commented May 11, 2016

All libraries should work with advanced compilation, that is the idea of Cljsjs. But yes, it is possible that some packages have outdated externs. One reason is that we don't have easy way to test the externs (#7). Some of these packages might have worked previously but have since then been updated to later version without updating the extern. When merging PRs I try to check if extern has been updated and if the version increase is likely to change the API.

I think solution is just to create an issue for each library where the advanced compilation doesn't work.

One related idea I've been thinking about, would be to flag "properly maintained" packaged somehow. Maybe list their maintainers. For example, I'm looking after React and lately also Leaflet and React-leaflet.

@martinklepsch
Copy link
Member

martinklepsch commented May 11, 2016

I think another smoke test could be to just advanced compile something.
doing this we can detect if the externs are at least valid syntax.

@Deraen
Copy link
Member

Deraen commented May 11, 2016

That's true. Duplicate/missing vars seem to cause some problems.

@xcthulhu
Copy link
Contributor Author

xcthulhu commented May 13, 2016

Short of running a unit test suite, it's hard to determine if a extern file is written properly.

All libraries should work with advanced compilation, that is the idea of Cljsjs.

In that case we should take down BitAuth, Vega, eccjs, and maybe attempt to audit all of the rest.

Flagging the ones with issues might be a start, however, since developers seem to be building things around libraries only to find out rather late that they don't support advanced compilation.

@Deraen
Copy link
Member

Deraen commented May 13, 2016

Yes, proper tests are hard but we could at least detect syntax errors by loading the externs in Closure.

I think I can create a Magical Bash Script to check all the externs or just changed externs in CI.

Flagging the ones with issues might be a start, however, since developers seem to be building things around libraries only to find out rather late that they don't support advanced compilation.

Yes, flagging would be good start. Auditing all the packages is not feasible.

@Deraen
Copy link
Member

Deraen commented May 13, 2016

@Deraen Deraen closed this as completed Mar 10, 2017
@Deraen
Copy link
Member

Deraen commented Mar 10, 2017

All packages SHOULD work with advanced compilation, that is the reason why this project exists. If some package doesn't work, PR (or issue) should be created.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants