-
Notifications
You must be signed in to change notification settings - Fork 2.4k
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you have a sample deployment? Maybe @arthurevans or @katejeffreys should try the gulp/deploy process?
app/elements/demo-tabs.html
Outdated
}, | ||
|
||
// Call out to a global method defined by app.js | ||
_plunkerLaunched: function() { | ||
_hideEditorButton: function(srcDefined, prjoectPathDefined) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
_hideEditorButton
is a method AND a property?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oops
app/elements/demo-tabs.html
Outdated
|
||
_isStackBlitz: { | ||
type: Boolean, | ||
}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
_isPlunkr
and _isStackBlitz
unused
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah I forgot to rename srcDefined
to this etc.
app/elements/demo-tabs.html
Outdated
return (!src || src === ''); | ||
_srcProjectPathChanged: function(src, projectPath) { | ||
this._srcDefined = !!this.src; | ||
this._projectPathDefined = !!this.projectPath; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Feel like _srcDefined
and _projectPathDefined
aren't needed since they're just proxies for src/projectPath
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the process of removing them, but I'm going to need one that is of type boolean as the databinding will not work if I try
<div hidden$=[[!stringThatIsUndefined]]>
app/elements/stack-blitz.html
Outdated
@@ -0,0 +1,186 @@ | |||
<link rel="import" href="./stack-blitz-sdk.html"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
stack-blitz-sdk.html doesn't need to exist (not used elsewhere so doesn't need to be de-duped) - just have <script src="../js/sdk.umd.js"></script>
here
}.bind(this)).catch(onError); | ||
}.bind(this); | ||
|
||
this.importHref('/elements/stack-blitz.html', onLoad, onError, true); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💯 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
changes implemented
Hey everyone- StackBlitz co-creator here! Super excited to see this PR :) Just wanted to give y'all an update on the ETA for the various items outlined above from our end. With ngconf done and our Teleport feature out the door, we're finally able to focus on the next version of our bundler/dev server which solves all the 'critical' & 'nice to have' items. Specifically, we're going to enable serving the static JS files themselves without any transpilation done beforehand. We're realistically shooting to have this live on prod in ~4 weeks- would love to hear everyone's thoughts/if there's func in polymer CLI that'd be ideal to have on StackBlitz as well 🥂 |
@EricSimons TBH it's miraculous that bare module specifiers work out of the box for ES6 modules with no additional tooling. There is nothing really polymer-cli-specific that I can think of atm, but there are a few rough patches that we've been filing issues for. Perhaps we can sync up later sometime in VC to discuss this further? |
Sounds good! My email is eric at esft.com, so feel free to drop me a note and we'll get something on the calendar 👍 |
\o/ i will try to review this Thursday 26th |
package.json
Outdated
@@ -7,7 +7,8 @@ | |||
"node": ">=5.0.0" | |||
}, | |||
"scripts": { | |||
"postinstall": "bower install; gulp", | |||
"postinstall": "bower install; npm run generate-stackblitz; gulp", | |||
"generate-stackblitz": "cp ./node_modules/@stackblitz/sdk/bundles/sdk.umd.js ./app/js/sdk.umd.js; cp ./node_modules/@stackblitz/sdk/bundles/sdk.umd.js.map ./app/js/sdk.umd.js.map;", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we use a hosted version of the SDK instead? Not a huge fan of this one-off, manual copying from node_modules/.
<script src="https://unpkg.com/@stackblitz/sdk/bundles/sdk.umd.js"></script>
(referenced https://medium.com/@ericsimons/stackblitz-sdk-instantly-add-live-environments-to-your-docs-blogs-more-73dab05c51ae)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, I'll just need to add this to the service worker
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
spoke offline, decided it's best not to add SW
.gitignore
Outdated
dist | ||
build | ||
|
||
# Dev directories | ||
/app/bower_components | ||
/node_modules | ||
/app/node_modules |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't think this is created.
package.json
Outdated
@@ -22,6 +22,7 @@ | |||
}, | |||
"homepage": "https://www.polymer-project.org/1.0/", | |||
"devDependencies": { | |||
"@stackblitz/sdk": "^1.1.1", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reset this file and package-lock.json
.gitignore
Outdated
@@ -3,6 +3,7 @@ | |||
# Generated files/folders | |||
*.pyc | |||
app/css/* | |||
app/js/sdk* |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove this
app/elements/stack-blitz.html
Outdated
@@ -0,0 +1,187 @@ | |||
<link rel="import" href="../bower_components/polymer/polymer.html"> | |||
|
|||
<script src="https://unpkg.com/@stackblitz/sdk/bundles/sdk.umd.js"></script> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pin unpkg to a specific version to avoid breakages/extra redirect https://unpkg.com/@stackblitz/[email protected]/bundles/sdk.umd.js
gulpfile.js
Outdated
@@ -251,6 +251,7 @@ gulp.task('jshint', 'Lint JS', function() { | |||
return gulp.src([ | |||
'gruntfile.js', | |||
'app/js/**/*.js', | |||
'!app/js/**/sdk*', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
argh, one more thing to remove
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm just testing you... you pass
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code changes LGTM. Will defer to @arthurevans and/or @katejeffreys to test on their machines as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested, works beautifully on my machine. Thank you so much for this!!
The commit messages might be helpful to follow for review here.
What I've done:
I added inline demos for
3.0
. It uses stackblitz. See gif at the bottom of this PR for exampleadded thegenerate-stackblitz
npm command which copies it from node modules to a servable folder in the postinstallstack-blitz
element<stack-blitz projectPath="/path/to/project"></stack-blitz>
demo-tabs
element to implement stack-blitzstack-blitz
is lazy loaded1.0
and `2.0 examples3.0
docs to use this new stackblitz demo tabs3.0
samples to use the manifestUsage:
index.html
and anindex.js
manifest.json
at the root of that folder{ files: string[], dependencies: { [npmDepName: string]: string }}
src
which was used for plunkr links, useprojectPath
which is the path to the sample folder you just created (e.g./3.0/samples/custom-element
)editorOpenFile
to declare what file will be shown upon editor embedPerformance
3.0 quick-tour Chrome with service worker and cache on
first time page load
subsequent page load
Lighthouse
Lighthouse score went up to high 60s from low 60s as we removed the iframe demos and lazy load the editor.
Outstanding issues
Critical
Importing an element that usespolymer-legacy.js
causes compile error Cannot read property 'end' of undefined stackblitz/core#441Nice to have
writenode_modules
instead ofturbo_modules
Install dependencies into node_modules, not turbo_modules stackblitz/core#333Demo
Try it out! Link