npm package compatibility

[benallfree]

This PR proposes a few package.json changes for npm compatibility.

1. Rename to laravel-ziggy since the ziggy name is already taken on npm.
2. Hoist qs out of devDependencies, because (see 3 below)...
3. Modern bundlers (rollup, webpack, parcel) will look for a module directive as an es6 source, and the package.json spec advises to have a browser entry point as well.
4. Add a prepublishOnly hook to build scripts right before npm publish
5. Add a files directive to publish only necessary files to npm's registry

If this PR is accepted, please publish to npm (see #277)

[ankurk91]

@benallfree

I would suggest to add files property too
https://docs.npmjs.com/files/package.json#files

This will prevent other files from being published to registry

  "files": [
    "src/js",
    "dist"
  ],

We can add prepublishOnly script too, which will be called right before publishing to npm, and will ensure to compile the js

"prepublishOnly": "npm run build"

[benallfree]

@ankurk91 Thank you, done.

[jakebathman]

Hey guys, @bakerkretzmar and I are talking about this right now. We're going to be moving from Travis to GitHub Actions, and we're pretty sure that would allow us to automate the NPM publish process. That's been our main concern until now was having to maintain the NPM package.

We've not forgotten about this and appreciate the work you've put into it!

[bakerkretzmar]

This looks great @benallfree, thanks! Is there a reason you updated the "main" key to point to dist/js/route.min.js instead of src/js/route.js?

My understanding of main is that it's the first place node should look if someone does import route from 'ziggy' or require('ziggy'), so it should export a module. Wouldn't pointing main at the bundled and minified webpack output break that?

[ankurk91]

main should point to the dist in our case, ziggy has to run in browser environment, we are using babel and webpack to bundle our source files.

When someone would be consuming this npm package, they won't need to compile from source,.

Let me know, if you need more explanation.

[bakerkretzmar]

I'm going to stick with using src/js/route.js as the entry point for now. Based on my research, bundlers like Webpack will be able to tree-shake, transpile, and minify the source files correctly this way.

If we pointed it to dist/js/route.js or dist/js/route.min.js, and, for example, you were also requiring the qs package in your app, you would end up with the contents of that package included twice.

My understanding is that if you aren't using a bundler at all, the browser key we're adding here should cover you. If you absolutely need the dist build imported some other way, once the package is published on npm you can use unpkg or cdnjs or something to include it directly with a script tag.

Thanks again!

[ankurk91]

We should point the main to dist, see examples

/~https://github.com/laravel/echo/blob/master/package.json#L19
/~https://github.com/vuejs/vue/blob/dev/package.json#L5

src has ES6 syntax, Babel can convert ES6 to ES5, but most module bundler keep node_modules to exclude list for babel, Laravel Mix does the same.

Laravel Mix, will not convert ES6 to ES5 for this package.

Webpack official docs recommends to do the same.

https://webpack.js.org/guides/author-libraries/#final-steps

So our main entry should look like this, notice: it is not minified.

"main": "dist/js/route.js",

[benallfree]

@bakerkretzmar

If we pointed it to dist/js/route.js or dist/js/route.min.js, and, for example, you were also requiring the qs package in your app, you would end up with the contents of that package included twice.

That's not correct. Webpack, Rollup, Parcel, Microbundle, and all other bundlers I know of will prefer browser and module targets over main. See Rollup's loading order and Webpack's loading order.

@ankurk91 is correct that main should point to a bundled version, although it matter less and less since all modern bundlers will reach for the module or browser target version instead, first.

So either way, I assume Laravel Mix will pick up browser or module before it picks up main. Has anyone tried that?

Since Ziggy is never a cjs/node package, main should point to source that works in Ziggy's only intended environment: the browser. That means it should be a bundled version.

But again it's mostly academic since I don't think any modern bundler will ever resolve to the main target.

[bakerkretzmar]

@ankurk91 @benallfree you're right, we'll change this back.

I hadn't found that particular page in Webpack's docs when I was reading about this, and I didn't take a close enough look at Ziggy's own build config to see what we're targeting.

I really appreciate the follow-up and your patience, thank you!