Skip to content
This repository has been archived by the owner on Aug 3, 2024. It is now read-only.

Add compile step that bundles and compresses JS files #684

Merged
merged 7 commits into from
Sep 23, 2017
Merged

Add compile step that bundles and compresses JS files #684

merged 7 commits into from
Sep 23, 2017

Conversation

timjb
Copy link
Contributor

@timjb timjb commented Sep 22, 2017 •
edited
Loading

Also, manage dependencies on third-party JS libraries using NPM. This addresses the second point in #683.

Todo:

  • improve JS code using ES2015 features and switch to TypeScript

alexbiehl
alexbiehl reviewed Sep 22, 2017
View reviewed changes
Copy link
Member

@alexbiehl alexbiehl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @timjb! Exactly what I had in mind. Looking forward to this patch!

.ghc.environment.x86_64-darwin-8.2.1 Outdated
@@ -0,0 +1,60 @@
-- This is a GHC environment file written by cabal. This means you can
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please remove this file from the diff. We should probably put .ghc.environment.* to .gitignore.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed.

haddock-api/src/Haddock/Utils.hs Outdated
@@ -326,16 +326,10 @@ makeAnchorId (f:r) = escape isAlpha f ++ concatMap (escape isLegal) r


jsFile :: String
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you rename jsFile to something more meaningful? Now that we have several JavaScript includes this derserves a less generic name I think. How about haddockJsFile?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed.

@timjb
Add compile step that bundles and compresses JS files
c912d4c 
Also, manage dependencies on third-party JS libraries using NPM.
@alexbiehl
Copy link
Member

This patch changes the QuickJump interface. We must not forget to change hackage accordingly:

It needs to know how to load the new quick-jump.js on package pages. Knowing QuickJump would undergo some changes I implemented a versioning scheme in haskell/hackage-server@51d86a3#diff-bed07c0e9d3155a8916521a5bd83a710R258. Haddock generates a meta.json which hackage inspects and chooses which version of QuickJump it has to deal with.

Currently it decides on the haddock_version field from meta.json but I think we should introduce a quickjump_version field to meta.json. And just like the way interface files are versioned (c.f. https://github.com/haskell/haddock/blob/master/haddock-api/src/Haddock/InterfaceFile.hs#L86) we increment the version everytime a breaking change to the QuickJump interface happens. This way we are decoupled from the haddock version.

@timjb timjb changed the title [WIP] Add compile step that bundles and compresses JS files Add compile step that bundles and compresses JS files Sep 23, 2017
@timjb
Copy link
Contributor Author

timjb commented Sep 23, 2017

I don't see the need for a separate new version number. I believe that we can keep the QuickJump interface stable going forward. For instance, this pull request doesn't change the quickNav.init interface, it only changes the organization of the JS files (one minified, bundled file instead of three). If we ever need to pass an additional argument to quickNav.init we can just do so, without breaking backwards compatibility.

@alexbiehl
Copy link
Member

Looks good to me. Just one more thing: Please separate the QuickJump css from haddock css.

@alexbiehl
Copy link
Member

I will prepare the corresponding commit to hackage.

@timjb
Copy link
Contributor Author

timjb commented Sep 23, 2017

Just one more thing: Please separate the QuickJump css from haddock css.

Done. The layout bug should be gone when quick-jump.css is used together with the Hackage CSS.

@alexbiehl
Copy link
Member

Great! Looks good. Will merge as soon as travis is green.

@alexbiehl
Copy link
Member

Dang. I think you forgot to update the tests.

@timjb
Copy link
Contributor Author

timjb commented Sep 23, 2017 •
edited
Loading

Yep. I've sent an updated patch.

@alexbiehl alexbiehl merged commit e99aefb into haskell:master Sep 23, 2017
@alexbiehl alexbiehl mentioned this pull request Sep 23, 2017
Closed
4 tasks
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@alexbiehl alexbiehl alexbiehl left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@timjb @alexbiehl