Issue 172853004: [polymer] preserve order of scripts in deployment

Issue 172853004: [polymer] preserve order of scripts in deployment (Closed)

Created:
6 years, 10 months ago by Jennifer Messerly

Modified:
6 years, 10 months ago

Reviewers:
Siggi Cherem (dart-lang)

CC:
reviews_dartlang.org

Base URL:
https://dart.googlecode.com/svn/branches/bleeding_edge/dart

Visibility:
Public.

More Reviews

Description

[polymer] preserve order of scripts in deployment We were potentially leaving some scripts in head, but moving other things into body. FWIW, I'm not sure it's a good idea to be moving anything from head to body, but that's what we got for now. R=sigmund@google.com Committed: https://code.google.com/p/dart/source/detail?r=32842

Patch Set 1 #

Total comments: 1

Patch Set 2 : revert pkg/polymer/test/build/static_clean_test.dart #

Total comments: 5

Created: 6 years, 10 months ago

Download [raw] [tar.bz2]

		Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+103 lines, -12 lines)			Patch
	M	pkg/polymer/lib/src/build/import_inliner.dart	View		6 chunks	+47 lines, -10 lines	3 comments	Download
	M	pkg/polymer/test/build/import_inliner_test.dart	View		3 chunks	+56 lines, -2 lines	2 comments	Download

Messages

Total messages: 7 (0 generated)

Expand Messages | Collapse Messages

Jennifer Messerly

Another change on the path to polymer.js interop. (fwiw, we may want to double down ...

6 years, 10 months ago (2014-02-19 22:00:04 UTC) #1

Jennifer Messerly

https://codereview.chromium.org/172853004/diff/1/pkg/polymer/test/build/static_clean_test.dart File pkg/polymer/test/build/static_clean_test.dart (right): https://codereview.chromium.org/172853004/diff/1/pkg/polymer/test/build/static_clean_test.dart#newcode13 pkg/polymer/test/build/static_clean_test.dart:13: print(build); // suppress unused import warning. oops, this was ...

6 years, 10 months ago (2014-02-19 22:00:25 UTC) #2

Siggi Cherem (dart-lang)

lgtm https://codereview.chromium.org/172853004/diff/60001/pkg/polymer/lib/src/build/import_inliner.dart File pkg/polymer/lib/src/build/import_inliner.dart (right): https://codereview.chromium.org/172853004/diff/60001/pkg/polymer/lib/src/build/import_inliner.dart#newcode138 pkg/polymer/lib/src/build/import_inliner.dart:138: link.replaceWith(imported); not sure I follow, why not remove ...

6 years, 10 months ago (2014-02-20 03:50:55 UTC) #3

Jennifer Messerly

https://codereview.chromium.org/172853004/diff/60001/pkg/polymer/test/build/import_inliner_test.dart File pkg/polymer/test/build/import_inliner_test.dart (right): https://codereview.chromium.org/172853004/diff/60001/pkg/polymer/test/build/import_inliner_test.dart#newcode110 pkg/polymer/test/build/import_inliner_test.dart:110: '<script>/*third*/</script>' On 2014/02/20 03:50:55, Siggi Cherem (dart-lang) wrote: > ...

6 years, 10 months ago (2014-02-20 03:56:07 UTC) #4

Jennifer Messerly

https://codereview.chromium.org/172853004/diff/60001/pkg/polymer/lib/src/build/import_inliner.dart File pkg/polymer/lib/src/build/import_inliner.dart (right): https://codereview.chromium.org/172853004/diff/60001/pkg/polymer/lib/src/build/import_inliner.dart#newcode138 pkg/polymer/lib/src/build/import_inliner.dart:138: link.replaceWith(imported); On 2014/02/20 03:50:55, Siggi Cherem (dart-lang) wrote: > ...

6 years, 10 months ago (2014-02-20 03:59:08 UTC) #5

Jennifer Messerly

Committed patchset #2 manually as r32842 (presubmit successful).

6 years, 10 months ago (2014-02-20 04:17:35 UTC) #6

Siggi Cherem (dart-lang)

6 years, 10 months ago (2014-02-20 04:37:19 UTC) #7

Message was sent while issue was closed.

https://codereview.chromium.org/172853004/diff/60001/pkg/polymer/lib/src/buil...
File pkg/polymer/lib/src/build/import_inliner.dart (right):

https://codereview.chromium.org/172853004/diff/60001/pkg/polymer/lib/src/buil...
pkg/polymer/lib/src/build/import_inliner.dart:138: link.replaceWith(imported);
On 2014/02/20 03:59:08, John Messerly wrote:
> On 2014/02/20 03:50:55, Siggi Cherem (dart-lang) wrote:
> > not sure I follow, why not remove the link node?
> 
> replaceWith should remove it & replace it with the inlined contents
> 
> What we used to do: gather all the things (recursively from all imports) into
> "imported" DocFragment, then put that whole thing into beginning of head.
> 
> That's tricky because some tags like <script> aren't inlined, but we want them
> to have similar ordering. Also html5lib doesn't let us easily query for
"script"
> and "link" and get them back in order.
> 
> So what it now does: move everything into <body> first, where we want its
> contents to end up. Then, each inlining operation simply becomes replacing the
> <link rel=import> with its contents.
> 
> link rel=stylesheet (the method below) works that way too.

Sorry, I saw the added lines, and for some reason I didn't notice that it used
to be a field before. Thanks for the awesome explanation though, it makes
perfect sense now.

Expand Messages | Collapse Messages