basecamp / trix

A rich text editor for everyday writing
https://trix-editor.org/
MIT License
19.1k stars 1.12k forks source link

NPM Module not working #119

Closed antoniobrandao closed 8 years ago

antoniobrandao commented 8 years ago

If I try to load Trix like regular NPM modules like this:

require('trix')

I get this error:

Uncaught ReferenceError: Trix is not defined       trix.js:20

However if I try to load it in the traditional way it works:

 <script type="text/javascript" src="trix.js"></script> 

But it would pain me to create this exception in my Browserify-based project. I'm loading everything else from NPM with "require".

It seems that the trix NPM module is not ready to be "required"? Isn't the point of having things in NPM, to have the ability to load it like a regular NPM module? Looking at the code I don't see the typical initialisers usually found in NPM modules (like the UMD pattern https://github.com/umdjs/umd).

Are there plans to enable this? Thanks.

mattrasband commented 8 years ago

Having the same issue, if there's anything you need from me to help with addressing this - please feel free to reach out.

bbenezech commented 8 years ago

This might help:

npm i -S script-loader
require('script!trix')

// global Trix is available
mattrasband commented 8 years ago

@bbenezech That appears to be for webpack only, do you know of any browserify compatible alternatives?

The suggested way for browserify is to use browserify-shim, but I am currently on a deadline so will for now just do it the ugly way and require it in the index.html as I am out of time.

antoniobrandao commented 8 years ago

Wrapping the source in the UMD fashion would solve the problem and make it compatible with all environments... https://github.com/umdjs/umd

To be specific, this one: https://github.com/umdjs/umd/blob/master/templates/returnExports.js

This approach is very popular nowadays.

bbenezech commented 8 years ago

@mrasband Sorry I missed the Browserify-based project part. I've never used Browserify.

mattrasband commented 8 years ago

No worries! Web pack looks promising for a future project

gscottolson commented 8 years ago

:+1: for UMD

chrisvfritz commented 8 years ago

I also tripped on this. I'd much prefer an API more like Highlight.js, where very simple projects can inject a global and get to work quickly with something like:

<script src="/path/to/trix.js"></script>
<script>Trix.initEditorsOnLoad()</script>

And then in more complex, modular applications, UMD would make this possible:

import Trix from 'trix'

const myInput = document.getElementById('my-input')
Trix.addEditor(myInput)

The latter is much more natural in component-based projects using React/Riot/Vue.

javan commented 8 years ago

Trix exports a single global Trix object. If you're using a module loader like Browserify or Webpack then you'll need to configure it accordingly. See https://github.com/turbolinks/turbolinks#installation-using-webpack for a Webpack example.

antoniobrandao commented 8 years ago

@javan Why not UMD?

lemonmade commented 8 years ago

It does seem strange not to choose UMD for this; it makes it pretty easy to use this package in any environment, without relying on the presence of plugins that simply do what UMD does anyways. We are using Trix and a custom asset bundling tool that currently has no good way of including the library in our bundle.

javan commented 8 years ago

Hi all,

I don't want to make Trix difficult for people to use in their applications and I'm not opposed to providing a module for convenience. @lemonmade proposed a reasonable change (https://github.com/basecamp/trix/pull/236) and the comment I added to that thread should improve things.

Trix is built for the browser and fully embraces its offerings: native DOM manipulation APIs, custom elements (document.registerElement), and powerful utilities like MutationObserver, TreeWalker, and requestAnimationFrame. None of these things are available in non-browser environments like node.js, which is why Trix is not packaged for them. Browsers do not support module.exports, define, or require, and introducing them to Trix means introducing the first 100% non-browser-compatible code to its codebase.

We are using Trix and a custom asset bundling tool that currently has no good way of including the library in our bundle.

Placing <script src="trix.js"></script> in your document is all it takes to add Trix your site, and if you have no way to accomplish that then it's probably worth examining your choice of tooling. Tools like webpack brought module support to the browser to make sharing code between the server and browser possible, and that's a good thing. But please don't assume these tools are always doing it The Right Way. Embrace the web.

mitar commented 8 years ago

Hm, but NPM is used also to package client-side code.

Moreover, if you really want to embrace the web, maybe think about providing Trix as a ES2016 module? ES2016 provides ways to import and export modules. Then other package builders can build on top of that.

In short, at least something (instead of making Trix symbol global) should be provided. And of course by default it could continue to write to global scope, but for large apps it is nice to keep things tidy.

javan commented 8 years ago

@mitar https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/export:

screen shot 2016-05-11 at 7 40 31 am
mitar commented 8 years ago

Yes, so you can use transpilers:

Note: This feature is not implemented in any browsers natively at this time. It is implemented in many transpilers, such as the Traceur Compiler, Babel or Rollup.

lemonmade commented 8 years ago

@javan I don't think anyone is advocating for any movement away from being a good web citizen. However, for large applications, serving every script via a dedicated script tag is simply not a good idea for performance. Just looking at our application's front-end dependencies, there are many fabulous, web-technology-using/ -dependant projects that allow usage both in the simple "include this script" situation, and in a more complex asset bundling setup (some examples: jQuery, FastClick, clipboard.js, NProgress).

javan commented 8 years ago

@lemonmade, I didn't mean to suggest that you should include a separate script tag for Trix. Although it would be perfectly reasonable to do if you're using HTTP/2. The point I was trying to make is that all a build tool needs to do is append / concatenate trix.js with its bundle. And from what I've gathered, they all make this simple task of joining strings into something complicated and confusing, and instead prefer browser-incompatible export APIs.

antoniobrandao commented 8 years ago

@javan You made really great points, and I understand your concern, but still, it wouldn't hurt to just add UMD to it. It wouldn't change anything in it's specific implementation. But it would greatly help the package managers connected to places where you published the module, like NPM.

It is very confusing to have a module in NPM, that cannot be loaded by Browserify. Whatever is in NPM, people expect to be able to require("trix") in a Browserify setup - if it's a module for the browser. That's where UMD will help, because then it could be loaded anywhere.

You don't have to add node-specific, non-browser-code into the codebase, to make it UMD. It's just a simple wrapper that will make your module more open to different loaders and environments (AMD, CommonJS, etc).

javan commented 8 years ago

@antoniobrandao Like I said, I don't want Trix to be difficult for people to use. We're looking into UMD support. Thanks for listening to me rant. 😁

javan commented 8 years ago

You don't have to add node-specific, non-browser-code into the codebase, to make it UMD.

Every module definition in UMD is non-browser-code. Yes, they're only used when defined, but no browser supports them natively.

antoniobrandao commented 8 years ago

@javan Thanks for the clarification there at the end - and for listening to me plea :)

We're in the age of package managers, using Browserify for example is a bliss, so I can keep track of my dependencies in the package.json. It's a nuisance to have to create an exception for any module, to include it in the SCRIPT tag of the HTML or whatever.

Perhaps a cool solution would be to have a specific implementation (UMD) in the module published to NPM - and on the other hand keep the main distribution package (from the site) simple, with browser-only code? I trust you guys to make the best decision :) glad you're looking into UMD. 👍

antoniobrandao commented 8 years ago

@javan btw I'm friggin lovin' Trix, kudos for this gem.

mitar commented 8 years ago

If I am reading this correctly, it seems there was a green light for UMD! :-) I also think that is the best approach for now and it will make it work well for consumers of the library. I am so happy about that.

And I agree, it should be much easier to do this in a "proper" way. But for now I think this is the best and most friendly approach what the ecosystem has.

javan commented 8 years ago

I opened a PR that adds UMD support: https://github.com/basecamp/trix/pull/241. It's not published on npm yet, but there's a download linked in the PR you can use. I'd appreciate it if you could give a try and let me know if works with your build tool of choice.

antoniobrandao commented 8 years ago

@javan will try it asap!