-
-
Notifications
You must be signed in to change notification settings - Fork 35.4k
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
Examples: Revive JSM -> JS Rollup script #20527
Conversation
cool as. turns out folks were looking into this before. keep up the good work. |
This is a bit strange, should rollup be deduping these?
This also improves life somewhat for people using NodeJS, or older bundlers like Browserify. But I'm also OK with the IIFE output. Not sure if Rollup would let us get the best of both with a footer like this?
|
It definitely could but it seems that rollup creates a new argument for every dependency no matter what. If you don't supply a global name for a dependency rollup "guesses" with a unique global name based on the dependency file name.
I don't think we can get away from some kind of iife function wrapper even if it did (which I don't think rollup affords this kind of control anyway). Consider a module that does the following: const tempVec = new Vector3();
function helperFunction() {
// ...
}
export class ExampleClass {
// ... ExampleClass uses helperFunction
} With just a footer |
No objection to the presence of an IIFE wrapper, I agree that it's probably the minimum wrapper for a generated file here. My argument is more that I prefer the functionality of UMD — really just its CJS support — while preferring the readability of IIFE. I was hoping that a suffix like the one I suggest above could just be appended to the IIFE output to get the best of both. EDIT: I guess it's not quite so simple when considering imports, too... |
Oh I see -- yes I agree. Unfortunately I don't think rollup has an option to build an IIFE and Commonjs without AMD. I'll have a closer look at some of the capabilities of the rollup plugins to see if they afford some more control over something like this. |
It doesn't look like Rollup has any support for custom output wrappers right now. There have been requests and discussions related to adding support for custom wrappers (rollup/rollup#2190, rollup/rollup#1326, rollup/rollup#269) but it seems interest hasn't been strong enough to add them. Other suggestions involve using regex or something like acorn to process the bundle before output. I toyed around with a Plugin for replacing iife argumentsrenderChunk: function( code ) {
let finalCode = code;
// regex for parsing the iife function arguments and passed parameters list
const argsRegex = /\(([^(]+?)\)/;
const passedArgsRegex = /\(([^(]+?)\)([^(]+)$/;
// get the iife arguments and parameters list
const args = code.match( argsRegex )[1].split( /,/g ).map( a => a.trim() );
const passedArgs = code.match( passedArgsRegex )[1].split( /,/g ).map( a => a.trim() );
// map with argument name to passed parameter names
const argsToPassedMap = new Map();
const finalArgs = new Set();
args.forEach( ( a, i ) => {
argsToPassedMap.set( a, passedArgs[ i ] );
} );
// remove exports argument because we will add it first as "THREE" manually
argsToPassedMap.delete( 'exports' );
// convert the argument names to human readable ones and replace the original names
// with the human readable ones through out the code
argsToPassedMap.forEach( ( value, key ) => {
if ( value === 'THREE' ) {
finalCode = finalCode.replace( new RegExp( key, 'g' ), 'THREE' );
finalArgs.add( 'THREE' );
} else {
// TODO: handle libraries and add them to args list
}
} );
// Delete 'THREE' because we add it manually at the beginning of the arguments list
finalArgs.delete( 'THREE' );
const finalArgsArray = Array.from( finalArgs );
finalArgsArray.unshift( 'THREE' );
// replace the iife arguments and passed parameters lists with our new deduped and human
// readable names lists
finalCode = finalCode
.replace( argsRegex, `( ${ finalArgsArray.join( ', ' ) } )` )
.replace( passedArgsRegex, `( ${ finalArgsArray.join( ', ' ) } )$2` )
return finalCode.replace( /exports/g, 'THREE' );
} Line2 output with IIFE post processing/**
* Generated from 'examples/jsm/lines/Line2.js'
*/
(function ( THREE ) {
'use strict';
var Line2 = function ( geometry, material ) {
if ( geometry === undefined ) geometry = new THREE.LineGeometry();
if ( material === undefined ) material = new THREE.LineMaterial( { color: Math.random() * 0xffffff } );
THREE.LineSegments2.call( this, geometry, material );
this.type = 'Line2';
};
Line2.prototype = Object.assign( Object.create( THREE.LineSegments2.prototype ), {
constructor: Line2,
isLine2: true
} );
THREE.Line2 = Line2;
}( THREE )); |
/cc #20529
Yes, probably worth getting input from @mrdoob here before spending too much time on any particular path. We'll also need to decide whether we actually want to generate /js versions of features that currently exist only in /jsm. For example, ZSTDDecoder, KTX2Loader, and NodeMaterial. |
Hello @gkjohnson, how to use this Rollup script? I want to auto convert my pr #20798 from
Yes, it should fail, because judging from the file name and path, this is a build config file and cannot be run directly. So then I ran How to use this Rollup script to auto convert |
@gonnavis I just added a |
@gkjohnson Thanks! Let me try. |
@gkjohnson How about also auto-generate What's your work flow of updating and testing |
@gonnavis I don't plan to work on this anymore until there's a more explicit direction for the conversion. If it's suggested that this approach looks good and / or needs tweaks or testing I can put time into whatever's needed. Regarding testing I'm confident in the results of Rollup -- it's a well maintained tool that's widely used and performs minimal transformations to the code. That's not to say testing shouldn't be done (at the least my small plugin functions need to be validated) but I think converting every html example is overkill and an unnecessary effort. I think we can just test a few of the most complex cases by hand. |
@gkjohnson OK, thanks again for your pr! |
Success! Only the one |
Closing in favor of #21584. |
Reviving the rollup script from #15526 / #15543 with some changes to convert the
examples/jsm
files toexamples/js
and address #20455 so we can start editing the module examples directly.This addresses some of the issues I brought up in #20455 (comment) by only building files that already exist in the examples/js directory and making those files interdependent if needed. If a shared module is not already present in the js directories then it is inlined in the dependent file. This means in some cases there could be redundant code bundled across multiple files if a module is shared but it keeps the resulting structure clean (and backwards compatible). In order to add a file that is not already being built we just have to create an empty file in the appropriate spot and rebuild.
Using UMD also addresses the concerns brought up in #17220 by enabling the built scripts to be used as commonjs imports and therefore function in code sandbox, though this might bring more confusion and I don't believe solves a problem anywhere else.
The remaining issues that have to be worked out:
The files in /libs need some special handling to properly use a global variable that is not
THREE
-- this will probably require hand-maintaining a list of lib -> global variable mappings but fortunately that list is short.The files are not as clean as the previous versions in part because of the UMD syntax (which can be removed) making hand editing them less viable. Even if we stick to just global variables rollup will wrap the files in an iife and generated variable names. With some fancy regex it might be possible to clean this up automatically. One thing to keep in mind is that we may not be able to get rid of the function wrapper because the modules may declare functions that could pollute the global namespace otherwise.
You can see an example of an output file with dependencies below. I'll await feedback / direction before putting more time into this. cc @mrdoob @Mugen87 @donmccurdy
Example output for Line2.js in IIFE format
Example output for Line2.js in UMD format