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 operator' code differs: UMD vs. node_modules #3850

Open
syndicatedshannon opened this issue Jun 21, 2018 · 4 comments
Open

'import operator' code differs: UMD vs. node_modules #3850

syndicatedshannon opened this issue Jun 21, 2018 · 4 comments

Comments

@syndicatedshannon
Copy link

syndicatedshannon commented Jun 21, 2018

To import an operator from UMD (our 'production' environment), we use:

import { operators } from 'rxjs';
const { filter } = operators;

To import an operator from node_modules (our 'development' environment), we use:

import { filter } from 'rxjs/operators';`

Operators are bundled in the main UMD package, but seem to be 'grafted on' as an additional root-level export only present for the UMD. They are not exported from /src/index.ts on 2018.06.22, so the UMD import method shown above is not available during development of our application.

Conversely, node's folder traversal method of module resolution is not available for the UMD, so the node_modules deep import method shown above isn't available at production run-time.

This means we have to modify our code to work correctly between development and production. Our production code presents resolution errors at development time and will not run against non-bundled RxJS, and our development code will build but fail in production. Such preprocessor differences are workable for bootstrap/entry-point code (which can have conditional substitutions applied fairly easily); but this import difference presents problems as it appears throughout compiled code.

Please correct the above if incorrect. I have a rather poor grasp on module loaders.

If the above is true, I believe one possible solution is to provide a separate UMD bundle for rxjs/operators. This would reflect the current RxJS 6 documentation (and also happens to support the current Angular 6 code). Using it with SystemJS would require one additional map statement, or no additional map statements in a certain matching CDN layout.

This issue is related to the discussion about optimal UMD bundling strategies here: #3463. However, this issue is specifically about the defect caused by lack of parity between the UMD and node_modules scenarios; currently the UMD does not actually work as expected.

@syndicatedshannon syndicatedshannon changed the title operator import code different: node_modules vs. UMD operator import code differs: UMD vs. node_modules Jun 22, 2018
@syndicatedshannon syndicatedshannon changed the title operator import code differs: UMD vs. node_modules 'import operator' code differs: UMD vs. node_modules Jun 22, 2018
@syndicatedshannon
Copy link
Author

syndicatedshannon commented Jun 27, 2018

FYI, our interim solution for the broken UMD (to simplify) is to hack the bundle, lifting all the operators into the root namespace:

const fs = require('fs-extra');
const path = require('path');
const uglify = require('uglify-js');

main("node_modules/rxjs/bundles/rxjs.umd.js", "compiled/rxjs.umd.min.js");

async function main(source, target) {
	if (await needsCompile(source, target)) {
		await build(source, target);
	}
}

async function build(source, target) {
	var dirReady = fs.ensureDir(path.dirname(target));
	var map = target + '.map';

	var src = liftOperators((await fs.readFile(source)).toString());
	var devReady = fs.writeFile(target.replace('.min', ''), src);
	var minified = uglify.minify(src, {
		sourceMap: {
			filename: path.basename(target),
			url: map
		}
	});
	
	await dirReady;
	await Promise.all([
		devReady,
		fs.writeFile(target, minified.code),
		fs.writeFile(map, minified.map)
	]);
}

function liftOperators(code) {
	var lines = code.split("\n");
	lines.splice(9107, 0,
	`for(var key in operators) {
		if(exports[key] == null) exports[key] = operators[key];
	}`);
	return lines.join("\n");
}

async function needsCompile(source, target) {
	var nc = await fileModified(source) > await fileModified(target);
	console.log(`${nc ? "out-of" : "up-to"}-date (source: ${source}, target: ${target})`);
	return nc;
}

async function fileModified(path) {
	if (await fs.exists(path)) {
		var stats = await fs.stat(path);
		return new Date(stats.mtime);
	} else {
		return new Date(0);
	}
}

@syndicatedshannon
Copy link
Author

Also wanted to mention. I appreciate the work towards availability of a publicly-maintained UMD bundle, and I also appreciate it's another build and maintenance step, and lastly I appreciate that it's already been quite a bit of effort just to organize the source code in a manner that makes a robust module-loader bundle strategy possible in the first place.

If you want help with this, please let me know and I'll do what I can. Module loaders and publishing to CDN are definitely not my strong suit, but our team depends on RxJS and would do our best to give back.

Thanks!

@mmacfadden
Copy link

We also have run into this issue. This is particularly problematic when dealing with bundlersLike rollup and webpack. We are publishing a library with an external dependency on RxJS. Did been published as an ES6 Module, a CommonJS Module, and a UMD. However getting all three to work from the same build is difficult because of this issue.

@BioPhoton
Copy link
Contributor

Maybe I'm wrong, but I believe it is already fixed.
@cartant or @benlesh please correct me, or, If it is fixed close it please

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

3 participants