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

Add UMD support. Fixes #91 #101

Merged
merged 5 commits into from May 23, 2016
Merged

Add UMD support. Fixes #91 #101

merged 5 commits into from May 23, 2016

Conversation

@javan
Copy link
Member

@javan javan commented May 17, 2016

Define UMD module for Turbolinks to support script / module build tools like webpack, browserify, require.js, etc.

@sstephenson
Copy link
Contributor

@sstephenson sstephenson commented May 17, 2016

We’ll need to update the documentation too.

@javan
Copy link
Member Author

@javan javan commented May 17, 2016

What do you think about removing the Installation Using Webpack documentation since it Just Works now?

@sstephenson
Copy link
Contributor

@sstephenson sstephenson commented May 17, 2016

I’m fine with removing the Webpack-specific parts, but we should still have a parallel set of instructions for using Turbolinks as a module. You still have to install the package and require it somewhere.

@javan
Copy link
Member Author

@javan javan commented May 17, 2016

Updated the docs

@sstephenson

This comment has been minimized.

Copy link
Contributor

@sstephenson sstephenson commented on README.md in 3ad2068 May 17, 2016

Both the iOS and Android adapters, as well as the Rails plugin, depend on Turbolinks being available in the global object. We should either tell people to assign to window.Turbolinks here or do it automatically in the module.

This comment has been minimized.

Copy link
Member Author

@javan javan replied May 17, 2016

Hmm, probably best to do that in index.coffee: @Turbolinks = window.Turbolinks = { ... }

This comment has been minimized.

Copy link
Member Author

@javan javan replied May 17, 2016

I'm fine with adding it to the npm docs too. Have a preference?

This comment has been minimized.

Copy link
Member Author

@javan javan replied May 17, 2016

Went the documentation route: b938b3b

@javan
Copy link
Member Author

@javan javan commented May 17, 2016

FYI, I confirmed the compiled .js works normally with <script src="turbolinks.js"></script> and with webpack and browserify:

(function() {
  var Turbolinks = require("./turbolinks");
  console.log("local var Turbolinks =", Turbolinks); // defined
})()

addEventListener("DOMContentLoaded", function() {
  console.log("widow.Turbolinks =", window.Turbolinks) // undefined
})
@TSMMark
Copy link

@TSMMark TSMMark commented May 19, 2016

As a Browserify user, I thank you! 👍

@sstephenson sstephenson merged commit 44a645f into master May 23, 2016
@sstephenson
Copy link
Contributor

@sstephenson sstephenson commented May 23, 2016

Thanks for making this possible @javan! 💝🎉

@sstephenson sstephenson deleted the umd branch May 23, 2016
@vipulnsward

This comment has been minimized.

Copy link

@vipulnsward vipulnsward commented on README.md in 3ad2068 May 27, 2016

@javan @sstephenson this is no longer needed now?

This comment has been minimized.

Copy link
Member Author

@javan javan replied May 27, 2016

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

Successfully merging this pull request may close these issues.

None yet

4 participants