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

[Megathread] Refactor ProjectConverter to Fix URL Handling #2721

Closed
7 tasks done
FredKSchott opened this issue Nov 7, 2017 · 2 comments
Closed
7 tasks done

[Megathread] Refactor ProjectConverter to Fix URL Handling #2721

FredKSchott opened this issue Nov 7, 2017 · 2 comments

Comments

@FredKSchott
Copy link
Contributor

Resolves #202 #203 #184 (#200 maybe, needs to be confirmed)
Implements #213

The PRs (Reviewers, Start Here)

PRs 1-5 should only take a few minutes to review each. They have been split out from the main PR to preserve your sanity.
PR 6 contains the meat of the implementation.

  1. Pin @babel/plugin-transform-classes to v7.0.0-beta.35 #216 1. Fix outputs keying to prevent cache missing, duplicates
  2. [cli] Do the script srcs need to be changed in polymer-3.x element template? #219 2. Refactor the top-level CLI & package/workspace conversion setups
  3. Scan exported functions #220 3. Refactor the url-converter.ts logic into the urls/ subdirectory
  4. [wct] Improving TypeScript configuration #221 4. Split out ConversionSettings from ConversionMetadata
  5. [wct] TypeScript: enabling strict mode #222 5. Add lookupDependencyMapping()
  6. [wct] Moving from clang + tslint to gts #224 6. Split out UrlHandlers (Package, Workspace) from ProjectConverter
  7. [wct] Update chalk to the latest version #225 Add conversion option to import npm packages by name, not path

You can see/compare/check out the final branch here: workspace-converter-refactor-8

Background

Polymer Modulizer was originally built to be run on a single repo at a time, with dependencies installed within that repo. The Modulizer would follow a packages dependency imports, convert those files, and then create new JS imports that pointed to the expected location of the converted dependencies on NPM.

When "Workspace mode" (the ability to convert multiple repos at once) was added, dependencies were installed as siblings of the main repos under conversion. This allowed a more efficient workspace installation (dependencies shared across repos) and a more efficient analysis (less files to analyze, entire workspace could be analyzed at once).

Unfortunately, the original code had been hardcoded to specifically expect dependencies as bower_components/ children and not siblings directories. Workspace mode was added without updating this expectation. Instead, entire workspaces were converted as if they were a single package, with every file being local to one-another.

This work-around allowed us to quickly publish alpha versions of most polymer elements, but has gone on to cause several bugs (specifically around dependency loading and import formatting) and block many new features (smarter import formatting, supporting names vs. paths, etc).

Goals

  • Properly analyze & convert "Workspace" layouts, specifically wrt dependency handling
  • Remove all hardcoded, package-layout-specific URL logic from DocumentConverter, conversion runners, etc.
  • Move all layout URL logic & responsibilities to a single place, if possible.

Design Overview

This design refactors the conversion coordinating "middle" layer of the Modulizer. It avoids any major changes to the higher-level CLI interface or the lower-level document specific conversion logic.

ConversionSettings

interface ConversionSettings {
    readonly namespaces: Set<string>;
    readonly excludes: Set<string>;
    readonly includes: Set<string>;
    readonly referenceExcludes: Set<string>;
    readonly referenceRewrites: Map<string, estree.Node>;
    readonly npmImportStyle: 'name' | 'path';
}
  • The static configuration properties of ConversionMetadata have been pulled out and refactored into ConversionSettings.
  • ConversionSettings stores static, readonly options for each conversion. For example: which namespaces to include, which files to exclude, etc.
  • It is a standard interface over user-provided options, leveraged as needed by the rest of the conversion process.

UrlHandler

interface UrlHandlerInterface {
    getPackageNameForUrl(url: OriginalDocumentUrl): string;
    getPackageTypeForUrl(url: OriginalDocumentUrl): PackageType;
    isImportInternal(fromUrl: ConvertedDocumentUrl, toUrl: ConvertedDocumentUrl): boolean;
    getNameImportUrl(url: ConvertedDocumentUrl): ConvertedDocumentUrl;
    getPathImportUrl(fromUrl: ConvertedDocumentUrl, toUrl: ConvertedDocumentUrl): string;
    convertUrl(url: OriginalDocumentUrl): ConvertedDocumentUrl;
}

class PackageUrlHandler implements UrlHandlerInterface { ... }
class WorkspaceUrlHandler implements UrlHandlerInterface { ... }
  • All the hardcoded expectations for how Analyzer URLs are handled, processed, understood, loaded, and formatted into imports have been pulled into a generic UrlHandlerInterface
  • A PackageUrlHandler is used for single package layouts, where all dependencies are located
    within the package directory.
  • A WorkspaceUrlHandler is used for workspace layouts, where all dependencies are located as siblings of each other.
  • By understanding the package->dependency relationship, UrlHandlers contain all the layout-specific logic needed for proper document conversion.
  • All other pieces of Polymer Modulizer can be layout-agnostic by delegating to a specific UrlHandler.

ProjectConverter

class ProjectConverter {
    readonly urlHandler: UrlHandlerInterface;
    readonly conversionSettings: ConversionSettings;

    convertDocument(document: Document): void;
    convertDocumentToJs(document: Document, visited: Set<OriginalDocumentUrl>): void;
    convertDocumentToHtml(document: Document, visited: Set<OriginalDocumentUrl>): void;
    private handleConversionResult(newModule);
    getResults(): Map<ConvertedDocumentFilePath, string | undefined>;
}
  • By leveraging a layout-specific UrlHandlerInterface implementation, ProjectConverter can be written without the need for extension.
  • The conversion logic previously split across BaseConverter, AnalysisConverter, and WorkspaceConverter has all been consolidated into a single ProjectConverter class.
@FredKSchott FredKSchott changed the title Refactor Project Converter to Add URL Handling Refactor Project Converter to Fix URL Handling Nov 7, 2017
@FredKSchott FredKSchott changed the title Refactor Project Converter to Fix URL Handling Refactor ProjectConverter to Fix URL Handling Nov 7, 2017
@FredKSchott FredKSchott changed the title Refactor ProjectConverter to Fix URL Handling [Megathread] Refactor ProjectConverter to Fix URL Handling Nov 7, 2017
@FredKSchott
Copy link
Contributor Author

All PRs have been merged, thanks for the reviews everyone!!!

thank you!

@stramel
Copy link
Contributor

stramel commented Nov 16, 2017

Great work @FredKSchott! 🎉

@aomarks aomarks transferred this issue from Polymer/polymer-modulizer Jan 3, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants