fix(requirejs): temporary fix for build

[zewa666]

this temp fix is for until we figure out what is causing gulp to lookup the index.js in the respective dependencies root-folder vs dist/commonjs

[zewa666]

@ghiscoding I've created this PR on top of your PR in the slickgrid-demos repo due to access restrictions. It's related to the quick-fix mentioned in ghiscoding/aurelia-slickgrid#466

[ghiscoding]

Thanks, I added you as a collaborator here as well, that's probably why you couldn't push directly.

[ghiscoding]

That's strange, I know the commonjs entry works because I use within the Slickgrid-Universal monorepo when it builds and when it uses the d.ts files for the Typings. So I'm not sure why RequireJS would have problem detecting it, here's the package.json of the common package

{
  "name": "@slickgrid-universal/common",
  "version": "0.5.1",
  "description": "SlickGrid-Universal Common Code",
  "browser": "src/index.ts",
  "main": "dist/commonjs/index.js",
  "module": "dist/es2015/index.js",
  "typings": "dist/commonjs/index.d.ts",
  "type": "module",
  "author": "Ghislain B.",
  "license": "MIT",
  "publishConfig": {
    "access": "public"
  },
  "files": [
    "/dist"
  ]
}

Do you see anything wrong with it? I guess you could try to modify the package.json directly on your side to see what change might work... I wonder if it's because of the relative path and try to add a slash in front /dist/commonjs/index.js

[ghiscoding]

I wonder if @3cp can help understand what the problem might be?
@3cp what would be the best way to troubleshooting such RequireJS path problem?

To give you a bit of a background, I'm switching Aurelia-Slickgrid to use a common code monorepo structure called Slickgrid-Universal and I believe the main entry point is correct (the link to the package.json is in the previous comment), I'd really like this to work as intended for RequireJS (it works fine with WebPack, only rjs seems to have problem with it). Does rjs read all the other entry points like module and browser? I hope that doesn't conflict with the main entry.

[zewa666]

I think all is fine from the package json as the main points to the proper file. I havent yet debugged into how dependencies are requested by the underlying gulp build Task, perhaps there is just a simple thing left. But @3cp definitely can add more insights faster. I'll be able to tackle this on Monday. The leading slash might be such an indicator

[3cp]

I will have a look tomorrow.

[3cp]

I checked @slickgrid-universal/excel-export, its package.json has "browser": "src/index.ts" but src/ files are not present in the published npm package, so it falls back to use index.js.
The fix is to remove "browser": "src/index.ts".

[3cp]

Bundlers (webpack and also cli-bundler) by default use "browser", "module", "main" fields in package.json in that order. In this case, "browser" field is picked up.
Webpack might fall back to next field in this case, cli-bundler did not do that graceful fall back.

[zewa666]

Ahh very good catch. Not even sure if its a worthy improvement to the CLI but perhaps a good doc. I'll try to document the order somewhere suiting in the CLI docs

[ghiscoding]

hmm good catch but I can't really change the browser entry point. The reason I'm using it is because that is the only working solution to get my VSCode chrome debugger extension to break properly in the Slickgrid-Universal monorepo and if I use only main then it stops working. I lost couple days on trying to make it work with jut main but couldn't find anything as good as using browser.

I think it works as I expect it to work on my side because I have this WebPack config in which Prod will default to main because browser is not part of it's list (it is only available for non-Prod). See this WebPack - mainFields doc.

resolve: { 
  mainFields: production ? ['module', 'main'] : ['browser', 'module', 'main']
}

Are there other alternatives to fix this? I don't want to lose my VSCode debugger in Slickgrid-Universal

[ghiscoding]

But isn't the bundler (RequireJS) suppose to go to the next available entry point, which is main (or module) not index.js? I believe that is what WebPack does and that is why I don't see the issue when using WebPack in Aurelia-Slickgrid-Demo while RequireJS complains. index.js should always be the last resource (entry), not the second, so I'm wondering why isn't RequireJS following that too?

[3cp]

Yes, I can and should improve cli-bundler to fall back gracefully.

On the other side, I don't know much about vscode debugger, but hijacking "browser" field seems to be weird solution.

[ghiscoding]

That's unfortunately the best I could find to get the debugger working with direct access to the source file, I couldn't get it working properly from the sourcemap.

That indeed would be great if the CLI bundler could fall back gracefully from browser to main then module and finally index.js.

[3cp]

Running into trouble for aurelia-cli because of the existing coupled code structure. It was an easy fix for dumber bundler.

[3cp]

Another relatively easy option for you would be to create two custom scripts for "prepublish" and "postpublish".

• remove "browser" field (it's invalid anyway for the final npm package) before publish.
• restore "browser" field after publish so your debugger would still work.

[ghiscoding]

Yeah I actually had that in mind, I was hoping that you could fix the CLI but if it's too much work (or not possible) then I'll look at modifying the package.json. I'm not using Gulp though but it should be doable with plain NodeJS code. Thanks for looking into this @3cp.

[3cp]

I haven't put more effort into cli fix. It requires some level of refactoring. If you can patch the package.json for publishing, pls do it.

Previously I had small motivation to enhance dumber core functionality so cli can reuse it and remove the outdated cli bundler code. That would solve more cli bundler known issues which were fixed by dumber design. But I never made the decision to pursue the enhancement because of the work load and the status of the going-to-be outdated aurelia 1 cli.

[zewa666]

@ghiscoding perhaps you can open a new issue describing how you're doing the Debugging including vscode settings file. Then we could try to fix the core issue instead.

As @3cp mentioned it really doesn't make a lot of sense to fix the CLI as its going to be obsolete with V2 and most users anyway go with Webpack. Plus there is this workaround merged and a documentation might be enough

[ghiscoding]

@zewa666 @3cp
So I took the approach that @3cp suggested to remove the browser before publishing and put it back after publishing and that was easy enough with a node script, it actually took me more time trying to figure how to have Lerna to stop complaining that it has git uncommitted changes when publishing, so this is what works (the addition of prepublishOnly and postpublish to modify the package.json files)

{
    "new-version": "lerna version --conventional-commits",
    "prepublishOnly": "lerna run package:remove-browser-prop",
    "publish": "lerna publish from-package --registry=https://registry.npmjs.org/",
    "postpublish": "lerna run package:add-browser-prop"
}

and you can see that browser is no longer part of the package.json, for example you can see the unpkg - @slickgrid-universal/common website.

BUT, that still doesn't fix the RequireJS build... I just don't understand, if I look at unpkg it's all good now, why isn't that working then? What I mean is that if I remove the Slickgrid-Universal dependency mappings (with this commit) that @zewa666 added in the aurelia.json file then RequireJS stops working but if I put them back then all is fine. I also don't understand why it seems to be pointing to aurelia-slickgrid.ts (TypeScript file), Aurelia-Slickgrid is also pointing to commonjs index.js file here and on unpkg, so why all of a sudden we are using TS file? but again this goes away after I put back the Slickgrid-Universal mappings.. so strange.

You guys can test it out with this PR and to get RequireJS to work, then rollback this commit

It shows auto-tracing in the command line without any errors though

INFO [Bundler] Auto tracing package: 0.7.7      @slickgrid-universal/common

Actually @slickgrid-universal/common seems to be the only problem here because that is the one main package used inside Aurelia-Slickgrid.

Could it be because I'm re-exporting everything with export * from '@slickgrid-universal/common'; on this line?
I do that so that all the interfaces and everything else declared in Slickgrid-Universal are usable for any users by calling the interfaces by doing import { Column, GridOption } from 'aurelia-slickgrid';, I really want this to work as it is because without this, everyone switching to newest version will have to refactor a lot of the code to point from Aurelia-Slickgrid to Slickgrid-Universal, so I would avoid this as much as possible

[3cp]

Note for npm package started with name aurelia-, cli bundler ignores "module" field, goes straight to "main" field.

https://github.com/aurelia/cli/blob/3fd32a6270afa5c87171fff7a1c88bbe0ef9fc4c/lib/build/utils.js#L197-L206

[3cp]

For @slickgrid-universal/common, the "main" field is a commonjs file, not esm file (nodejs doesn't use "module" field), so you should not have "type": "module" in its package.json.

[3cp]

Sorry, the aurelia- is a different thing than @slickgrid-universal/common, cli did use "module" field for @slickgrid-universal/common. I am investigating why the es2015 dist files have problem but your forced commonjs files work fine.

[3cp]

I added log to node_modules/@slickgrid-universal/common/dist/es2015/extensions/cellExternalCopyManagerExtension.js right before the error line.

The Slick doesn't have CellExternalCopyManager. I have no idea why the behaviour differs between your es2015 and commonjs files.

[ghiscoding]

But why is it trying to use the module? I thought RequireJS uses main (commonjs) by default? I'm a WebPack user so I don't know RequireJS well enough, I just assumed things. I didn't test the es2015 output and actually didn't think it was even going to be used.

to give a bit of a background, SlickGrid is an old jQuery lib, and Slick is a jQuery namespace.
Can you see if the Slick object is set or is it really the Slick.CellExternalCopyManager that is undefined?

[3cp]

Requirejs doesn't do anything, it doesn't know anything about nodejs npm system. It's the bundler (cli-bundler in this case) chose npm files.

For comparison, there is the same log same screenshot when running forced commonjs dist files.

CellExternalCopyManager is in Slick!

[3cp]

From the two screenshots, you can see CellExternalCopyManager is not the only thing missing in Slick when using es2015 dist files.

[ghiscoding]

oh I see it in your print screen, that is actually the Slick object and the CellExternalCopyManager object is missing which is why it fails. That is very helpful, it's most probably because I use require('plugin') on this line like require('slickgrid/plugins/slick.cellexternalcopymanager'); to load the plugin dynamically after the fact... I was thinking to replace that code with regular import since I thought require would help to get smaller bundler but I found out that it doesn't help at all and isn't tree shakable either. It looks like these require comes later and aren't part of the export of the es2015 bundle.

How did you get these kind of print screen? That is very helpful to look at it that way, is that an OS or an editor thing?

[3cp]

It was the standard screenshot tool provided by macOS.

[ghiscoding]

What would happen if I remove the module from the package.json will it force it to use the main instead? If so, that could fix the problem isn't it? I mean at least until I replace the require code by regular imports

[3cp]

It looks like your dynamic required plugins are those missing ones. :-)

[3cp]

Yes, you can remove module for an easy fix. You just lose the tree-shaking in webpack which only works on esm.

[ghiscoding]

Yes indeed, I have been considering for some time to refactor that, so that is an even more of a push to do it.
oh ok so if I follow you, is es2015 what is considered to be esm?

Thanks for everything, this is really, really helpful. Can't say it enough 😉

[3cp]

esm is the EcmaScript Module format.
es2015 is EcmaScript 2015 syntax standard, which uses esm format (import and export) by default.

You can setup tsc to emit esm code and still target older browser (didn't support 2015 syntax). For example, aurelia 1 core modules have dist/es2015 and dist/native-modules, the native-modules is esm with older js syntax, es2015 is esm with newer js syntax.

[3cp]

You are welcome.

[ghiscoding]

@3cp
I just wanted to give you a quick update, in case you were interested... Everything is fixed and it all works in latest version of Aurelia-Slickgrid

• I got rid of the browser line in the final code pushed to NPM (with scripts to delete it from package.json when versioning while keeping it internally for VSCode debugger)
• I also fixed the missing imports (changed all require(...) to regular imports from ...
• Also fixed couple of other issues found with RequireJS and ES6 (MomentJS is imported differently in ES6 vs TS)

So the end result is that I managed to get rid of all the mappings that were previously necessary and now most of the packages are auto-detected by the CLI. We're back to business 😉

I wanted to thank you again for all the time you spent in helping me achieve better support with better ESM builds as well

Cheers and happy year end. 🎄
Also thanks for all the Aurelia work you do 😄

[3cp]

Sounds great! Same thanks for you too, as you too are doing open source to help others.