Skip to content
This repository has been archived by the owner on Jan 4, 2019. It is now read-only.

Javascript Bundling Mega-PR #637

Merged
merged 95 commits into from
Mar 27, 2018
Merged

Javascript Bundling Mega-PR #637

merged 95 commits into from
Mar 27, 2018

Conversation

usergenic
Copy link
Contributor

@usergenic usergenic commented Mar 6, 2018

This PR adds ES6 Module Bundling to Bundler.

…l modules include the relative path to the module from the bundle entrypoint.
…ing PR feedback. Made the bundle type key off the presence of a subbundle URL in entrypoints list.
…orts instead of nesting them under namespace keys.
…ently applied to all es6-module bundles via same analysis.
 - Renamed `_inlineScripts` to `_inlineNonModuleScripts`
 - Renamed `_rollupInlineScripts` to `_inlineModuleScripts`
   - Moved responsibility for re-analyzing AST inside
     `_inlineModuleScripts` since that is implementation detail
     of that algorithm needing a `document.getFeatures` call.
 - Moved `rewriteNode` from `babel-utils` to `overwriteObject` in
   `utils.ts` since it is now used in two places.
…ing change in an array of file contents, but order was not important. Fixed a test.
@usergenic
Copy link
Contributor Author

@justinfagnani PTAL; feedback addressed.

Also, has been brought up to date with master.

@aomarks
Copy link
Member

aomarks commented Mar 23, 2018

still LGTM

* delegate only to Analyzer.
* https://github.com/Polymer/polymer-bundler/issues/646
*/
export function parseModuleFile(url: string, code: string): babel.File {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add issue to remove this parse and use Analyzer.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed parseModuleFile and using Analyzer :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Which also meant I could remove babylon and @types/babylon

* Within the `root` of the babel AST, find and returns a NodePath of the
* given `node`. Returns `undefined` if node not found within `root`.
*/
export function getNodePath(root: babel.Node, node: babel.Node): NodePath|
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's check if this file is a good candidate to move into Analyzer

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed function completely - see next comment.

undefined {
let nodepath;
traverse(root, {
noScope: true,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't we traverse up path.parentPath? That would turn O(size_of_subtree) to O(length_of_path)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just pass in the NodePath now so I don't need to use getNodePath to find it. So fixed!

@usergenic usergenic merged commit 84a2e4e into master Mar 27, 2018
@usergenic usergenic deleted the rollup-js-bundles branch March 27, 2018 22:43
@stramel
Copy link

stramel commented Mar 28, 2018

🎉 Nice work @usergenic 👍

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants