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

Fix Sourcemaps #515

Merged
merged 23 commits into from
Nov 14, 2017
Merged

Fix Sourcemaps #515

merged 23 commits into from
Nov 14, 2017

Commits on Nov 10, 2017

  1. Default source map should come after Directive Processor

    The directive processor can add a newline to assets. If it comes after the default source map then this line is not accounted for when determining line offsets.
    schneems committed Nov 10, 2017
    Configuration menu
    Copy the full SHA
    45f01e0 View commit details
    Browse the repository at this point in the history
  2. [close #388] Do not add newline when appending Semi-colons

    Adding a new line with semi-colons can cause off by 1 errors with source maps. Instead we want to add a semicolon. If the last character was whitespace such as a newline, we want to make sure that there are the same number of `str.lines` before and after, so we replace the whitespace character with a semicolon followed by the same whitespace character.
    schneems committed Nov 10, 2017
    Configuration menu
    Copy the full SHA
    1d28403 View commit details
    Browse the repository at this point in the history
  3. Whitespace

    schneems committed Nov 10, 2017
    Configuration menu
    Copy the full SHA
    870945b View commit details
    Browse the repository at this point in the history
  4. Fix source map concatenation

    The calculation of length of asset is done by counting semicolons, however not all mappings end with a semicolon. When they do we must increment the offset.
    
    The next part is harder to explain. 
    
    We concatenate two assets A.js and B.js. A has 10 lines B has 5 lines.
    
    After we’ve done this we want an offset that looks like this:
    
    ```
    A.js: {"line"=>0, "column"=>0}
    B.js: {"line"=>11, "column"=>0}
    ```
    
    To get 11 we take the prior length of mappings for A.js which is 10 (from the lines) and increment by one to get to 11.
    
    Now we add another asset C.js which has 2 lines. We want an offset that looks like this
    
    ```
    A.js: {"line"=>0, "column"=>0}
    B.js: {"line"=>11, "column"=>0}
    C.js: {"line"=>16, "column"=>0}
    ```
    
    If we add the last offset which is 11, to the length of the asset of B (which is 5) then we get the correct offset which is 16. We do not have to add an extra +1. 
    
    I think this was accidentally working previously based on unwanted behavior from adding semi-colons and newlines.
    schneems committed Nov 10, 2017
    Configuration menu
    Copy the full SHA
    11217f9 View commit details
    Browse the repository at this point in the history

Commits on Nov 13, 2017

  1. Preserve original map if available

    This patch preserves an original source maps if one was present.
    schneems committed Nov 13, 2017
    Configuration menu
    Copy the full SHA
    66c2bd7 View commit details
    Browse the repository at this point in the history
  2. Length is faster than count

    ```
    Warming up --------------------------------------
                  count
       317.318k i/100ms
                  length   329.512k i/100ms
                  size     329.953k i/100ms
    Calculating -------------------------------------
                  count       9.806M (± 7.7%) i/s -     48.867M in   5.014627s
                  length     11.696M (± 6.3%) i/s -     58.324M in   5.008040s
                  size       11.074M (± 7.5%) i/s -     55.102M in   5.005626s
    ```
    schneems committed Nov 13, 2017
    Configuration menu
    Copy the full SHA
    e146451 View commit details
    Browse the repository at this point in the history
  3. Configuration menu
    Copy the full SHA
    ff5b6dd View commit details
    Browse the repository at this point in the history
  4. Cannot rely on semicolons in mappings

    Based on the spec it is not guaranteed that there will be a mapping for each and every line https://groups.google.com/forum/#!topic/mozilla.dev.js-sourcemap/gp_ULp-h1fQ
    
    We were previously using this behavior to know the length of a previous asset when concatenating source maps. 
    
    We can get around this by storing the length of the previous asset in an “x_sprockets_linecount” key. This is guaranteed to be on the asset since it is added by the `DefaultSourceMap`.
    
    
    We no longer need to offset the first asset by one line. This was accidentally working before.
    schneems committed Nov 13, 2017
    Configuration menu
    Copy the full SHA
    033c6b6 View commit details
    Browse the repository at this point in the history
  5. Fix tests for concat_javascript_sources

    We no longer add a newline if one did not previously exist.
    schneems committed Nov 13, 2017
    Configuration menu
    Copy the full SHA
    30b5838 View commit details
    Browse the repository at this point in the history
  6. No newline after semicolon

    schneems committed Nov 13, 2017
    Configuration menu
    Copy the full SHA
    3a0c297 View commit details
    Browse the repository at this point in the history
  7. Configuration menu
    Copy the full SHA
    21fb47b View commit details
    Browse the repository at this point in the history
  8. Configuration menu
    Copy the full SHA
    9d808a4 View commit details
    Browse the repository at this point in the history
  9. Fix comment

    schneems committed Nov 13, 2017
    Configuration menu
    Copy the full SHA
    b4f2dfa View commit details
    Browse the repository at this point in the history
  10. Configuration menu
    Copy the full SHA
    8ad78ba View commit details
    Browse the repository at this point in the history
  11. Configuration menu
    Copy the full SHA
    25d1fa8 View commit details
    Browse the repository at this point in the history
  12. Configuration menu
    Copy the full SHA
    809ecc9 View commit details
    Browse the repository at this point in the history
  13. Deep dup without refinements

    schneems committed Nov 13, 2017
    Configuration menu
    Copy the full SHA
    b238342 View commit details
    Browse the repository at this point in the history

Commits on Nov 14, 2017

  1. More robust concurrent test

    Previous test was failing intermittently with results like `["0", "0", "1", "1", "0", "0", "1", “1”]`
    schneems committed Nov 14, 2017
    Configuration menu
    Copy the full SHA
    f2bbb41 View commit details
    Browse the repository at this point in the history
  2. Test offsets

    schneems committed Nov 14, 2017
    Configuration menu
    Copy the full SHA
    1c8756a View commit details
    Browse the repository at this point in the history
  3. Bump cache version

    We need this because `concat_source_maps` expects a `x_sprockets_linecount` key in the map metadata. It wasn’t put there until this PR, so if a map gets pulled out of the old cache and attempts to get loaded into memory then it will fail.
    
    By rev-ing the cache we can ensure the key will be there, since it’s added by `DefaultSourceMap`.
    schneems committed Nov 14, 2017
    Configuration menu
    Copy the full SHA
    3b13ef8 View commit details
    Browse the repository at this point in the history
  4. Configuration menu
    Copy the full SHA
    e59e64d View commit details
    Browse the repository at this point in the history
  5. Index into source instead of buf

    So we can take advantage of the UTF32 encoding.
    schneems committed Nov 14, 2017
    Configuration menu
    Copy the full SHA
    3e75ea2 View commit details
    Browse the repository at this point in the history
  6. Add comment explaining logic

    schneems committed Nov 14, 2017
    Configuration menu
    Copy the full SHA
    b7f530b View commit details
    Browse the repository at this point in the history