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

Build it #3

Closed
matt-gadd opened this issue Jul 7, 2016 · 8 comments
Closed

Build it #3

matt-gadd opened this issue Jul 7, 2016 · 8 comments
Assignees
Labels
Milestone

Comments

@matt-gadd
Copy link
Contributor

matt-gadd commented Jul 7, 2016

We should be able to production-ify our Todo-MVC application

@matt-gadd
Copy link
Contributor Author

Posting the notes from building with r.js so far:
Sourcemaps back to original source

Problem: r.js still doesn't support a per file -> per input sourcemap to uglifyjs
requirejs/r.js#470
Workaround: resolved reasonably with https://github.com/Rich-Harris/sorcery tracing of sourcemaps post-build

Bug: TypeScript removes EOF's:
microsoft/TypeScript#3287
(if tsc didn't do this, the below bug wouldn't happen)

Bug: r.js appends a semicolon to the last line of a file regardless of whether it's a comment, results in a sourcemap url with a semicolon, which sorcery doesn't like.
requirejs/r.js#799
Fix: with skipSemiColonInsertion: true, but would affect modules that don't insert semicolons

Bug: maquette's source map specifies an incorrect sourceRoot of /sources/ which does not exist so cannot be resolved by sorcery
Fix: by manually injecting maquette's map for /source/maquette.js in sorcery. Get maquette to fix their map.

Bug: built file loaded in dojo/loader results in a circular dep detection for dojo-has.
Fix: temporarily by removing paths from config, which shouldn't fix it, but does

Bug: Output sourcemap for sorcery has the wrong source paths for all the dojo2 packages (relative path loss)
Fix: TBD

@matt-gadd
Copy link
Contributor Author

@kitsonk kitsonk added this to the 2016.08 milestone Jul 19, 2016
@matt-gadd
Copy link
Contributor Author

matt-gadd commented Aug 11, 2016

Okay I now have a working Webpack build at: https://github.com/matt-gadd/examples/tree/build-webpack

The Webpack build drops the need for grunt in todo-mvc, the TS is compiled via ts-loader in Webpack, any resources are copied via Webpack also.

Unfortunately their were a fair few hoops to jump through due to how we currently ship our dependencies:

Bug: Webpack does not understand the TS UMD wrapper at all. This results in Webpack not being able to deduce any of the dependencies even when forcing amd or cjs modes.

Workaround: Had to write a nasty AST mangler to trick Webpack into resolving the deps. https://github.com/matt-gadd/umd-compat-webpack-plugin/blob/master/index.js. This causes some other minor issues further down the line (we end up having to stub a require no-op in the built file)

Bug: Webpack generates built files with syntax errors for AMD style dynamic requires it can't resolve: webpack/webpack#920. In our case blows up at https://github.com/dojo/core/blob/master/src/load.ts#L57.

Workaround: Turn off evaluation of dynamic require expressions.

Problem: Because we need to alias our deps to the dist/umd folder, Webpack can't find an index.js for non file specific based requires ie require('dojo-has')

Workaround: ResolverPlugin.FileAppendPlugin allows us to specify a default file to check, so we can point to main.js (the AMD default) in the dist/umd dir

Problem: Sourcemap remapping for libs are supported by the source-map-loader plugin (equivalent to sorcery that I used for rjs). Maquette with its slightly broken sourcemap blows it up.

Workaround: For now, limit to the dojo deps only.

Bug: Sourcemap source filepaths are messed up for dependencies.

Overall I think this shows up the limitations of shipping UMD. The commonjs side of things is not ideal as the shape of the actual package is not commonjs like making it difficult to consume out the box. For all the issues that we have with Webpack and UMD I imagine other tooling like rollup (and it's transformer for cjs to imports) will have the same issues.

Given the issues, we should at least seriously consider shipping a proper commonjs style package - that way Webpack et al will work without any configuration or hacks.

Finally, we need to seriously consider making a decision on what we tooling officially support and recommend, this may impact our work on say css-modules or any other part of the build pipeline other than simply building JS. And if we officially chose Webpack, it raises some high level issues on what happens to dojo-loader, intern support etc.

@kitsonk
Copy link
Member

kitsonk commented Aug 11, 2016

Did you do any research into ESM and WebPack? Would that be a better "final" format for bundling?

Yes, I agree that we should publish a shape of a package just for CJS. I think it is the only way we will get around the challenges of not being able to re-root CJS easily and if we output the /src to / in the package and rewrite the package.json we could produce something that works. I agree it means we also should abandon the UMD as well.

@matt-gadd
Copy link
Contributor Author

@kitsonk WebPack 2.0, which is still in beta has support for ESM (although i'm unsure of how well it sticks to the spec, statements like webpack differs from the ES6 modules spec: imports are not hoisted don't encourage me 😂). I can certainly take a look though?

@kitsonk
Copy link
Member

kitsonk commented Aug 11, 2016

Naw, less adventure, more productivity (maybe).

@rishson rishson added the todoMVC label Sep 2, 2016
@rishson
Copy link
Contributor

rishson commented Oct 4, 2016

@matt-gadd status is still in-prog?

@matt-gadd
Copy link
Contributor Author

@rishson I think we can close this.

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

3 participants