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

Issue when loading Knockout via AMD #9

Closed
jstclair opened this issue Aug 27, 2014 · 2 comments
Closed

Issue when loading Knockout via AMD #9

jstclair opened this issue Aug 27, 2014 · 2 comments

Comments

@jstclair
Copy link
Collaborator

Hello!

We have an issue when loading Knockout via require; editableCell fails to load, since it expects ko to be a global (i.e., window) variable.

I've created a minimal reproduction of the problem here: https://github.com/jstclair/ec-amd-bug

After cloning the repo,

  • Ensure you have the right npm globals installed:
npm install -g gulp typescript bower http-server 

The last one is optional, but I'm going to assume it's installed later.

  • Install local and bower deps
npm install
bower install
  • Build the site using gulp
gulp
  • Start http-server for the src/ folder (will work because of our tremendous hack)
http-server src/

Open a browser to http://localhost:8080/

EditableCell is working! This, however, is due to the preEditableCell.js script (in the src/vendor folder) and a shim (in the src/app/require.config.js file).

  • Quit the previous http-server instance, and run it from the dist folder.
http-server dist/

Opening a browser, you will see it doesn't work.

Espen and I have cloned the editableCell repo and tried to change options in the browserify config in order to force editableCell to externally require knockout, but the only thing we've been able to accomplish is including the entire knockout.js script in editableCell (not our preferred solution).

Any help would be greatly appreciated!

@gnab
Copy link
Owner

gnab commented Aug 29, 2014

¡Hola!

I made the following changes to the develop branch:

  • knockout is now being required internally in editableCell (like you tried), and to prevent it from being included in the bundle, the -x knockout option is passed along to browserify, which required updating browserify to version 2.27.1 to get to work.
  • the browserify bundle is getting modified, replacing the AMD top-level define call from define(e) to define(['knockout'], e)
    • this makes RequireJS load knockout as a prerequisite, thus the RequireJS config must define a path for knockout (see index.html).
  • to still support the non-AMD, non-CommonJS scenario, instead of directly referencing knockout internally, a wrapper has been added that checks to see if the global ko is available or else calls require('knockout') (AMD).

Also the index.html now uses RequireJS to load everything.

I copied the new editableCell.js into your src/vendor folder and removed the preEditableCell file to confirm, and everything now works just fine when running from both src and dist.

@jstclair
Copy link
Collaborator Author

Cool, thanks!

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

No branches or pull requests

2 participants