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

Import Order in ES6 / Dependencies #4

Closed
SamThilmany opened this issue May 27, 2017 · 12 comments
Closed

Import Order in ES6 / Dependencies #4

SamThilmany opened this issue May 27, 2017 · 12 comments
Assignees

Comments

@SamThilmany
Copy link

I noticed this bug for the first time in the awps theme, but it was also very simple to reproduce it in a non WordPress-based project. If the import-function in ES6 is used like this,

import $ from 'jquery';
    window.$ = $;
    window.jQuery = $;
import 'izimodal';

one would expect, that all the imports other than jQuery, which may be dependent on the latter, are loaded correctly, since jQuery has already been defined before. Unfortunately, JS is not executed consecutively and so very often an error message appears in the console, which tell one, that the script XY needs jQuery:

iziModal requires jQuery

Probably it only works, if script A, that is dependent of script B, is bigger than script B, because then script A will be finished earlier with processing and thus be in front of script B in the resulting main.min.js. In my case, the code bit throwing the error was in line 8 of my main.min.js, so definitely before the jQuery code.

Here is the code bit of iziModal:

if (typeof jQuery === "undefined") {
     throw new Error("iziModal requires jQuery");
}

Does anyone have an idea how to handle this bug?

PS: In Alex' video, this bug is not noticeable, because WordPress integrates jQuery before loading other scripts and Alex did not unregister WP jQuery.

@Alecaddd
Copy link
Owner

Oopsie, what a silly developer I am 😥

I totally forgot about testing the jQuery import, and now that I see it properly, in ES6 the window variables are not accepted anymore, that's why other packages can't read it.

I need to update the gulp workflow to properly compile and package everything.

I'll fix it ASAP and push the updated version.
Thanks for flagging this out.

@Alecaddd
Copy link
Owner

Alecaddd commented May 30, 2017

@SamThilmany
Ok, it should be solved now.

What I had to do was implementing browserify-shim in order to properly import variables to be used globally.

If you pull the latest code you will notice that I'm using require instead of import for jquery, and declaring the $ variable

/**
 * Manage global libraries like jQuery or THREE from the package.json file
 */
var $ = require( 'jquery' );

For globally required variables, if you want to use my method in order to package and bundle all the scripts into a single one, you need to update the package.json file.

Here's the example related to jquery

"browser": {
		"jquery": "./node_modules/jquery/dist/jquery.js"
	},
	"browserify-shim": {
	    "jquery": "$"
	},

Here I'm declaring the global variable $ to use jquery from the node_modules folder.
I'm declaring the "jquery" alias in the "browser" declaration so I can use the require('jquery') without redeclaring again the full path of the file.

I also deregistered by default the built-in jquery version of WordPress.
Let me know if it works for you.

@Alecaddd Alecaddd added the bug label May 30, 2017
@Alecaddd Alecaddd self-assigned this May 30, 2017
@SamThilmany
Copy link
Author

@Alecaddd
Thank you very much for your quick changes.
Unfortunately it seems, that this does not do the trick (in my case at least).

The packages still yell at me, because they require jQuery. I tried with browserify-shim and browserify-global-shim while using your code or the one provided by the documentation of the package. In both cases 'jQuery' seems to be loaded too late...

If I manage to get it up and running, I'll definitely share it, but I don't mind if you keep you eyes open too. 😉

@Alecaddd
Copy link
Owner

Alecaddd commented May 30, 2017

@SamThilmany
Mhhhh, are you sure you pulled the updated code?

I left iziModal required in the main.js file and it doesn't trigger any error for me. Both jQuery and $ sign works inside imported libraries.

Also, did you run npm install again to get the new required packages?

@SamThilmany
Copy link
Author

Yeah, I did run npm install and I also think, that I updated all my files. I didn't pull though, because I'm currently using your gulp-code in a non-WordPress project.

I now added

"browserify-shim": {
    "jquery": "$",
    "main": {
        "depends": [
            "jquery"
        ]
    }
},

which seems to do the trick with jQuery, but now he tells me that he can't find the module:

Error: Cannot find module 'tether'

If I comment out tether, It's the next module which is included in main.min.css which cannot be found. This is really weird to me...

If you like to see the code, I just made a git repo. Note that I didn't use your latest version of the gulpfile, because I don't have an account.js and so I also don't need to map over several files.

@SamThilmany
Copy link
Author

I just set up a WordPress site for testing your latest awps theme and it works well, even if I don't understand why.
But, it only works using the iziModal package (and maybe some others, who knows). I now tried to include Bootstrap and guess what, there's a little issue here.

ReferenceError: Can't find variable: jQuery

@Alecaddd
Copy link
Owner

Thanks for the extra info, I'm glad it works on a standard WP installation (that was my main goal 😛), but it doesn't make sense not working externally, or with other libraries like Bootstrap.
I will investigate more and let you know!

@Alecaddd
Copy link
Owner

@SamThilmany Any updates on this issue?

If it works with WordPress and the theme, I would like to close it.

@SamThilmany
Copy link
Author

No, it still does not work. I just set up a new local site with your latest code by using your awps-cli. When adding import 'bootstrap' I still get the error message:

Error: Bootstrap's JavaScript requires jQuery. jQuery must be included before Bootstrap's JavaScript.

PS: The main.js now looks like this:

/**
 * Manage global libraries like jQuery or THREE from the package.json file
 */
var $ = require( 'jquery' );

// Import libraries
import 'bootstrap';
import 'tether';
import 'slick-carousel';
import 'izimodal';

// Import custom modules
import App from'./modules/app.js';
import Carousel from './modules/carousel.js';

const app = new App();
const carousel = new Carousel();

@Alecaddd
Copy link
Owner

I tested it and it works for me!

Here's what I did:

  1. Installed Bootstrap via npm
    npm install bootstrap@3
  2. Imported the compiled and minified CSS into my style.scss file
@import '../../node_modules/bootstrap/dist/css/bootstrap.min.css';
  1. Imported Bootstrap in my main.js file
import 'bootstrap';

Compiled everything with Gulp with the latest version of AWPS, the one with this configuration of browserify-shim inside package.json

"browserify": {
		"transform": [
			"browserify-shim"
		]
	},
	"browser": {
		"jquery": "./node_modules/jquery/dist/jquery.js"
	},
	"browserify-shim": {
		"jquery": "$"
	}

Everything works for me, including tooltips, modals, carousel, etc.

@Alecaddd Alecaddd removed the bug label Jun 21, 2017
@SamThilmany
Copy link
Author

SamThilmany commented Jun 21, 2017

Hmmm... I've exactly the same code 😕

Tested in Safari 10.1.1 (12603.2.4) and Chrome 58.0.3029.110 (64-bit).

The error message is Chrome is a bit longer:

Uncaught Error: Bootstrap's JavaScript requires jQuery. jQuery must be included before Bootstrap's JavaScript.
at Object.n.1 (main.min.js:1)
at o (main.min.js:1)
at main.min.js:1
at Object.n.6../modules/app.js (main.min.js:7)
at o (main.min.js:1)
at t (main.min.js:1)
at main.min.js:1

@Alecaddd
Copy link
Owner

Alecaddd commented Jun 21, 2017

Are you sure you have my same package.json file, with my same exact configuration?
If it's different, please use mine, delete your node_modules folder and run npm install again.

You also said that you're using my code outside a WordPress theme.
How did you update the gulpfile.js did something change there?
Be sure the JS compiling follows my same order of browserify and babelify.

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