Need to test with bundlers

[ericelliott]

Because the popular bundling tools may not bundle stuff from node_modules our package.json may need to point to bundled versions of the library, instead of individual modules. I've tried doing this with Browserify, but then Webpack gives weird warnings to all the users of our module (I think -- it used to, anyway).

We could try to accomplish this with rollup.

For this to be complete we need to test:

• [ ] npm install from the npm tar bundle
• [ ] Build and use in Webpack
• [ ] Build and use with Browserify
• [ ] Build and use with Rollup

cc @MarkHerhold @gmaclennan

[MarkHerhold]

Just to clarify the point on node_modules, we currently don't have any node_modules to bundle as this library only has dev dependencies in package.json.

@ericelliott, could you give a specific use case of someone running into an issue using cuid? I'm guessing this case wouldn't involve a build tool like webpack or browserify since these tools already bundle everything that is needed.

[gmaclennan]

I'm unclear about the exact nature of the bug? Both webpack and browserify should be fine with bundling with the main field pointing to index.js, although I have not tested webpack with the latest version. I am unclear about how Webpack respects the browser field in package.json - the docs state that it is supported but perhaps only as string values?

We could include the build files in the npm bundle by adding an empty .npmignore file (which would stop npm from ignoring the dist in the .gitignore - https://docs.npmjs.com/misc/developers#keeping-files-out-of-your-package) then users could require('cuid/dist/cuid'), however, I notice that we only build a node version, not a browser or react-native version. We should probably fix that.

Any feedback from users about what warnings or errors they are getting from Webpack would help resolve things.

[gmaclennan]

Ok I see the problem. The files field in the current package.json means that the lib folder is not being included in the tarball when published to npm, so everything will fail when installing from npm. Probably best just to remove that field, and use .npmignore to blacklist rather than whitelist files. I'm just heading out for the weekend now, so probably won't get round to doing this until next week.

FYI I tested the bundle without publish with npm pack then from a tmp local folder npm i ../cuid/cuid-2.0.0.tgz

[ericelliott]

FYI I tested the bundle without publish with npm pack then from a tmp local folder npm i ../cuid/cuid-2.0.0.tgz

I hadn't thought of that. Thanks for the tip!

[ericelliott]

Any progress?

[pyrossh]

We need to add fuse-box also as a bundler. Its not working since its trying to load the os package. Wouldn't it better if we have 2 packages. Anyways fusebox provides a global process env variable with these fields {title: "browser", browser: true, env: {…}}. Maybe we could use this to switch which packages are loaded. And also what about react-native, how would it run in that environment?

[ericelliott]

Good questions. Please explore and let me know what you figure out. =)

[notrab]

I'm having issues with using rollup and I was wondering if anyone here has had a similar issue?

The error I get back is Uncaught TypeError: n is not a function and the sourcemap reveals this to be a function that returns cuid().

If I replace cuid() with something else, it works.

Love to hear any suggestions @ericelliott 😄

[ericelliott]

@notrab

If the function returns the result of calling cuid(), that would be a string, not a function. We'd need more context to say much more. What does the function look like, and what is its intended purpose?

[nevf]

@ericelliott Same problem using parcel.js - "Its not working since its trying to load the os package."

[ericelliott]

I think our new browser tests have this covered?