-
-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Glimmer packages #12772
Glimmer packages #12772
Conversation
this will basically keep failing until we publish glimmer with dist built etc. |
'glimmer-util': glimmerPackage('glimmer-util'), | ||
'glimmer-wire-format': glimmerPackage('glimmer-wire-format'), | ||
'glimmer': glimmerPackage('glimmer'), | ||
'simple-html-tokenizer': glimmerPackage('simple-html-tokenizer') |
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.
This is duplicated. Do we need two different versions of SHT? If not, then removing the duplicate should be fine, if we do need different ones then we need to figure out how to namespace them to prevent clobbering.
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.
This is duplicated. Do we need two different versions of SHT? If not, then removing the duplicate should be fine, if we do need different ones then we need to figure out how to namespace them to prevent clobbering.
this is a @chancancode question :P
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.
Looks like we did some import changes in SHT: https://github.com/tildeio/simple-html-tokenizer/commits/master (htmlbars uses c4787e3 and glimmer uses 8b09fc2). If we can change htmlbars to work with the new structure then we can just release a new SHT version and use that in htmlbars (and do another htmlbars release?).
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.
Looking at the changes, it seems like it might Just Work™ in htmlbars too?
oops |
OK, just pushed a few changes:
Once glimmerjs/glimmer-vm#49 is landed, this is ready to merge (after confirming Travis is passing). |
fc6051a
to
5bf5bf6
Compare
'amd/glimmer-compiler.amd.js', | ||
/*'amd/glimmer-runtime.amd.js', */ | ||
'amd/tests.amd.js' | ||
] |
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.
This should be:
'amd/glimmer-common.amd.js', // stuff shared between runtime and compiler, e.g. utils
'amd/glimmer-compiler.amd.js', // for "server compilation", depends on common
'amd/glimmer-runtime.amd.js', // runtime stuff, depends on common
'amd/glimmer-tests.amd.js' // tests, depends on common, compiler and runtime
LGTM |
Restarted sauce labs tests... |
41e9bc3
to
cc15887
Compare
I plan to still improve this, but this should help move stuff long.
* import glimmers build… use it
…o we must transpile it. If we restructure the glimmer build slightly, we could just consume the separate AMD modules (likely as one would expect..)
We will lock this down when we have made more progress.
This version adds the `proto-to-assign` transform which makes static methods work on IE <= 10
cc15887
to
3d791dd
Compare
this is the "quickest" way to import glimmer into ember.... This is to allow work to begin, that being said. I plan to actually have them unify build-pipelines, as this should allow a much better development experience.