fix(scripts): allow using same lib inside app

[filipesilva]

We were loading scripts by adding an include list to the script-loader loader rule, forcing that list of files to always be loaded with script loader.

But there were cases where you could need to load it both via scripts (because other global scripts needed a lib), and inside your app (or libs your app imported).

This cased the import to return a empty object because it was loaded as a string due to being forced through script-loader.

With this fix, scripts are forced to be loaded via script loader individually for the scripts entry point instead of globally.

Fix #2141

[hansl]

Could you add a test that would fail before this change?

[filipesilva]

@hansl done.

[hansl]

So string-script will be BOTH in scripts.bundle.js and main.bundle.js?

[filipesilva]

Yes, but in different ways:

// in scripts.bundle.js
/***/ 600:
/***/ function(module, exports) {
module.exports = "console.log('string-script');\r\n"

// in main.bundle.js
/***/ 614:
/***/ function(module, exports) {
console.log('scriptJsContent');

Script loader modifies the content such that it is a string, which makes it useless for normal imports.

[hansl]

Looks to me that the real fix isn't this PR then. Can we get rid of script-loader?

I don't want people having 2 versions of, say, jQuery in their apps... we'll have tons of issues filed with that.

[clydin]

script-loader also causes CSP issues since it uses eval so removing it would be good in general.

Could the scripts be processed (concat, etc.) beforehand and then added as a webpack asset. or maybe a more extensive plugin or loader?

[filipesilva]

Adding them as an asset is an interesting idea. Maybe we can make a plugin that does that.

[hansl]

LGTM.

[tallaxes]

One side effect of this is that it is no longer possible to reference TypeScript files via scripts, as now script-loader has them eval'ed directly as string ...

[guzuomuse]

@filipesilva,the 2141
not fixed!
please check it,thanks.

compare these two:

    globalScripts.forEach(script => {
      let scriptPath = `script-loader!${script.path}`;
      entryPoints[script.entry] = (entryPoints[script.entry] || []).concat(scriptPath);
    });

vs

    globalScripts.forEach(script => {
      let scriptPath = `script-loader!${script.path}`;
      if (script.lazy) { lazyChunks.push(script.entry); }
      entryPoints[script.entry] = (entryPoints[script.entry] || []).concat(scriptPath);
    });

which one is right?

[filipesilva]

@guzuomuse why do you say this is not fixed? is there a scenario in your app where you get failures?

[guzuomuse]

@filipesilva thanks for replying;
yeah, there still not fixed in my opinion.
the ng -v

node: 6.9.5
os: win32 x64
@angular/common: 4.0.0-rc.1
@angular/compiler: 4.0.0-rc.1
@angular/core: 4.0.0-rc.1
@angular/forms: 4.0.0-rc.1
@angular/http: 4.0.0-rc.1
@angular/platform-browser: 4.0.0-rc.1
@angular/platform-browser-dynamic: 4.0.0-rc.1
@angular/router: 4.0.0-rc.1
@angular/cli: 1.0.0-rc.0
@angular/compiler-cli: 4.0.0-rc.1

in ".angular-cli.json"

 "scripts": [
        "../node_modules/jquery/dist/jquery.js",
        "assets/semantic/semantic.js"
      ]

and in app.component.ts

import { Component, OnInit,AfterViewInit,ViewEncapsulation } from '@angular/core';
import * as $ from 'jquery';


import 'slick-carousel';//just for testing other libray.
import 'jqueryui';//same with up.
......
export class AppComponent implements OnInit,AfterViewInit {
  
  ngOnInit() {
    $('#test_input_1').datepicker();   //here goes well with no problem
    $('.message .close')
      .on('click', function () {
        $(this)
          .closest('.message')
          .transition('fade'); //here goes wrong..
      })
  }
.....

and you can still get console error:

"EXCEPTION: WEBPACK_IMPORTED_MODULE_1_jquery(...).closest(...).transition is not a function"

[filipesilva]

@guzuomuse you code does not seem to work outside of the CLI, so I don't think it's a CLI bug.

See https://plnkr.co/edit/JHc8GOk5sBcQ6XWsHwJ9?p=preview for a repro of what you're trying to do in a plunker. Click the h1 and you will see the same error.

Most likely you're using the JQuery API incorrectly.

[guzuomuse]

@filipesilva , i don't think i have used jQuery API in a wrong way..
notice this :
.transition('fade')
it comes from ' "scripts": [
"../node_modules/jquery/dist/jquery.js",
"assets/semantic/semantic.js"
]'.
the function 'transition' defined in the semantic.js .
and Further talk:
if you delete "import * as $ from 'jquery' " and other related code except the 'transition' function .

everything goes well;

and further again. if you left the 'script:[]' empty ,just remove the 'transition' function in souce code.
it goes well too. no error occures.

that's say, if you include jquery in 'script:[..]',
and you can't use '$' with other libary which not included in the 'script:[]', (here is jquery ui datepick)
then can never coexist.

[filipesilva]

Ah so semantic is a jquery plugin? You never told me that 😃

Then it really depends on how that plugin functions unfortunately.

The global and local jquery instances are completely separate as that is the only way to ensure functionality.

When you add jquery to scripts it's exactly the same as adding it as a script tag in index.html. jquery is then on the global scope.

But when you import it via an import statement, you get a second local copy, that is not the same as the one on the global scope. This is intended by design, since ES6 modules are isolated.

The local copy does not have that semantic plugin. It's up to the plugin to provide import semantics compatibility. If that plugin only works by detecting jquery on the global scope then it cannot be used as a ES6 module.

For interop with packages that aren't meant to be used as ES6 modules your best bet is to work with the global scope. That means that you add jquery and all those plugins to the script array, and never import them in your TS code.

Then you create a src/typings.d.ts file and add declare var $: any there. If you have the real jquery type use that instead of any. That basically tells TypeScript to just assume there is a variable called $ in the global scope.

Then when you use it in your code you are using the global one and not a locally imported one. That global one has all the legacy jquery plugins you added.

[guzuomuse]

a,ha;
thank you very much ....

[jmls]

I was trying to solve this by creating a vendor.ts file .. but ... can't get it to work

import * as moment from 'moment';
window['moment'] = moment;

import * as jquery from 'jquery';
window['jQuery'] = window['$'] = jquery;

import * as _ from 'lodash';
window['_'] = _;

import * as Backbone from 'backbone';
window['Backbone'] = Backbone;

where should this file be placed ? And how should it be referenced ?

If I put it in scripts I get the error uncaught SyntaxError: Unexpected token import(…)

[guzuomuse]

@filipesilva .
if i didn't missunderstand. as you said:"

That means that you add jquery and all those plugins to the script array, and never import them in your TS code

".
it means we probably add many plugins into the 'script:[]'. but if by this way,the script.bundel.js will be very large size. isn't it?
if so ,that sounds not a good idea.
because
1: it will cost a lot of time to load at the first time.
2: most part of the 'script.bundel.js' functions weren't used. that should be loaded when required!

am i right?
thanks very much. if you can ,give me a feedback.... :)

[filipesilva]

You can't lazily load global scripts with import statements, no. If you want to lazy load them you have to find some other way to do it.

[clydin]

Your script bundle much like the vendor bundle should rarely change. As a result, it will most likely be cached for the user.
Also, components such as jquery will most likely be used throughout the app; which would cause lazy loading, of said components, to provide little overall benefit.

[guzuomuse]

thank you all kind guys.@filipesilva @clydin .
i think i almost got it :)

and at last i found another way:

in ts file,if some plugin featured es6,
using import "script-loader!somelibary",otherwise, import "somelibary";
that works fine for me 。
good luck.！

[clydin]

be aware that while that currently works, it is an unsupported capability and may not work in the future.

[angular-automatic-lock-bot]

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

_{This action has been performed automatically by a bot.}