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

Comment the webpack configurations #366

Open
deontologician opened this issue May 14, 2016 · 2 comments
Open

Comment the webpack configurations #366

deontologician opened this issue May 14, 2016 · 2 comments

Comments

@deontologician
Copy link
Contributor

They're very dense, and hard to understand for people wanting to go in and make quick modifications. It would be helpful if they were well documented.

@deontologician deontologician modified the milestones: Subsequent, Medium term plans, Release 1.x.x May 14, 2016
@deontologician deontologician modified the milestones: Next Major Release, Release 1.x.x Jun 4, 2016
@plievone
Copy link
Contributor

plievone commented Jun 16, 2016

@deontologician @flipace I could help with this. The build scripts may have changed, but a few months ago the client was built so that:

client/src -> client/lib was a simple source-to-source babel transform, no webpack, so that the package and its dependencies could be required directly in user's build process, and that one could later switch to using the src files directly if wanted (as that babel transform is just there to be able to use es6 syntax). It seems that is still the case, see package.json prepublish, scripts/compile.js and src/.babelrc. The main field in package.json points to lib/index.js, so the files are separate with proper stack traces and there's no polyfills. This also means that all dependencies need to be requireable from the filesystem, so that build tools such as browserify and webpack can use dependencies' main vs. browser package.json fields to provide a proper browser/node version, packagers can modify the dependencies by injecting their own module shims, packagers can dedupe dependencies across projects, etc.

It seems that this lib version (which is the default when requiring the library as a dependency) was broken by introducing some webpack specific directives to src, and it is understandable as the tests use only the webpacked version, not this plain lib version. This lib dir could be removed, if people are happy to use that webpacked dist version in their build processes, but I understood it is usually frowned upon and even warned in webpack ("This seems to be a pre-built javascript file. Though this is possible, it's not recommended. Try to require the original source to get better results."), although it is possible (see for example noParse as used to speed up test bundling by requiring the packaged files directly, no warnings then).

Going further, client/src -> client/dist had multiple targets which were bundled using webpack and babel loader, and that seems to be still the case, see webpack.config.js. There were versions with polyfills and without (by using a different entry file, src/index-polyfill.js, which could include Promises if they are needed), and with production mode and without (mostly minifying, but could also strip some debug warnings if kept in sync with src->lib step). Here the main complication comes as the same target bundles are used in browser and node testing, and some dependencies may behave differently when used/bundled in browser vs node (using http, fs etc). A simplifying solution could be to either (A) build separate webpack versions for browser and node (using webpack's target:'web', target:'node' config, with some additional config where needed), or (B) building only for browser with webpack in dist and just testing plain lib with node in CI. In both cases you should somehow ensure that browser bundle works too, as CI tests only in node for simplicity. You may also need a separate dev watcher for babel, as currently only dist is built with webpack when files change. Otherwise browser tests would not run via /test/serve.js as conveniently as now.

Currently the situation is fuzzy as webpack bundles are made requireable in node using a combination of externals and ProvidePlugin, so for example, webpack can inject runtime checks to test whether it is used via a script tag or as a CommonJS module, and thus can import some dependency from a global variable, or from the bundle, or from filesystem. When used in combination with packagers, this can become very hacky and convoluted quickly, as can be seen from my experiments here, here and here.

How would you like to proceed, what are the current use cases and obstacles? Some related issues are #600, #581, #571, #566, #509, #477, #420, #316, and #306. Do you want to keep the straightforward lib conversion that is referred to in package.json main? Do you want to switch to providing only the built bundle file, with perhaps two versions, one for node, one for browser? Do you want a separate polyfill entrypoint, or just document the need for polyfills? How do you want the RxJS dependency handled, what would be ideal? And also what are the implications for the CLI tool, and for serving the file from the horizon server?

@deontologician
Copy link
Contributor Author

Hey plievone!

Do you want to keep the straightforward lib conversion that is referred to in package.json main?

Yeah, we just need to maintain the "no webpack directives in require" invariant going forward (which we broke)

How do you want the RxJS dependency handled, what would be ideal?

This one is tricky, since we're dependent on a particular version of RxJS. Ideally RxJS would be a peer dependency, and we'd use :: to avoid modifying the Observable class the app developer is in charge of. In practice, for the near future we'll probably need to monkeypatch Observable, and for that it's better that we provide our own Observable class, and patch in a minimal subset of useful operators (we have plans to put in all of RxJs lite operators).

I think it'd be nice if we had different builds with different amounts of RxJS operators patched in.

And also what are the implications for the CLI tool, and for serving the file from the horizon server?

Theoretically, we could serve all different builds in the dist directory, so the user could decide which onese they want.

@deontologician deontologician modified the milestones: Next Major Release, Medium term plans Jun 21, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants