-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
CoffeeScript support for vue-cli and parcel templates #1467
CoffeeScript support for vue-cli and parcel templates #1467
Conversation
This is based off the work done by @zephraph on codesandbox#443 and is not quite yet working.
I have a <script lang="coffee">
import HelloWorld from "./components/HelloWorld"
export default
name: "App"
components: { HelloWorld }
</script> It does correctly get compiled by the CoffeeScript compiler and then injected into some code to But, the eval code looks like this: (function evaluate(require, module, exports, process, setImmediate, global, afterAll, afterEach, beforeAll, beforeEach, describe, it, test, expect, jest, __dirname, __filename) {
import HelloWorld from "./components/HelloWorld";
export default {
name: "App",
components: {HelloWorld}
};
}) The following error is shown when it runs: It sort of like the movie "Inception" to me, so I'm not really sure in which context there is an unexpected identifier. Is it complaining about the import statement? Or is something else fishy? |
literate: false, | ||
inlineMap: true, | ||
sourceMap: false, | ||
transpile: false, |
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 where we are supposed to supply the transpiler details. Something like this:
transpile: {
transpile: require('@babel/core'), // this doesn't work, since no require()
presets: ['@babel/env'], // this doesn't work, what should it say?
},
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.
Wouldn't it make sense to only use coffeescript for the transpilation from .coffee to .js and then run the Babel transpiler on that output?
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.
@CompuIves - Yes, I just couldn't figure out how to do it.
@@ -151,6 +152,9 @@ export default function initialize() { | |||
vuePreset.registerTranspiler(m => m.path.endsWith('pug'), [ | |||
{ transpiler: pugTranspiler }, | |||
]); | |||
vuePreset.registerTranspiler(m => m.path.endsWith('coffee'), [ | |||
{ transpiler: coffeeTranspiler }, |
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 think it would be solved by appending
{ transpiler: babelTranspiler }
to the array!
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 will try to get this to work!
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.
Actually, I tried this but it didn't work. Instead, I had to add const postLoaders = { coffee: 'babel-loader' };
to transpilers/vue/loader.js
, which made it all work.
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 tried this, but it didn't work. I added const postLoaders = { coffee: 'babel-loader' };
to eval/transpilers/vue/loader.js
and it worked!
@@ -148,9 +149,13 @@ export default function initialize() { | |||
{ transpiler: noopTranspiler }, | |||
]); | |||
vuePreset.registerTranspiler(() => true, [{ transpiler: rawTranspiler }]); | |||
vuePreset.registerTranspiler(m => m.path.endsWith('pug'), [ | |||
vuePreset.registerTranspiler(module => /\.pug$/.test(module.path), [ |
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 updated the pug
section also, just so all sections in this file use the same syntax.
bare: true, | ||
literate: false, | ||
inlineMap: true, | ||
sourceMap: false, |
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 removed the transpile: ...
entry here.
}; | ||
|
||
const loaders = Object.assign({}, defaultLoaders, codeSandboxLoaders); | ||
const preLoaders = {}; | ||
const postLoaders = {}; | ||
const postLoaders = { coffee: 'babel-loader' }; |
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 seemed like the clencher... only by having this could I get the <script lang='coffee'>
sections of the .vue
files to work.
This PR seems to make it so that However, when I import
What is needed to be able to import |
@@ -39,7 +40,7 @@ const getFileNameFromVm = vm => { | |||
export default function initialize() { | |||
const vuePreset = new Preset( | |||
'vue-cli', | |||
['vue', 'json', 'js', 'ts'], | |||
['vue', 'json', 'js', 'ts', 'coffee'], |
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.
Is adding coffee
necessary here? Seems to work with or without it.
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.
It's used to allow import './test'
to resolve to test.coffee
. I think it would be useful 😄
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.
Ok, cool.
@@ -127,6 +128,9 @@ export default function initialize() { | |||
vuePreset.registerTranspiler(module => /\.vue$/.test(module.path), [ | |||
{ transpiler: vueTranspiler }, | |||
]); | |||
vuePreset.registerTranspiler(module => /\.coffee$/.test(module.path), [ | |||
{ transpiler: coffeeTranspiler }, | |||
]); |
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 originally added this coffee
transpiler to the end of the file, but realized that it was not being executed since the level or declaration determines its priority and it was lower than the raw-loader
.
import { buildWorkerError } from '../utils/worker-error-handler'; | ||
|
||
self.importScripts( | ||
`${process.env.CODESANDBOX_HOST || ''}/static/js/coffeescript.2.3.2.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.
Would it be better to pull this from a CDN instead of including this always?
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.
Hmm, I think it doesn't really matter where we get it from. CDN works too, might be a bit faster even, but not noticeably I think.
@SaraVieira, @CompuIves - Does everything seem ok? If so, I believe it will be easy to add support for the parcel template also. |
Once coffeescript support was in for |
worker: Worker; | ||
|
||
constructor() { | ||
super('coffee-loader', CoffeeWorker, 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.
Does this 1
mean the number of concurrent workers? Should we change it or keep it at 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.
Yep! I think 1 is enough for the amount of coffee files in a project (since node_modules
doesn't have coffee
files it should be a low amount)
@@ -122,6 +122,7 @@ export default function(content: string, loaderContext: LoaderContext) { | |||
ts: ['ts-loader'], | |||
typescript: ['ts-loader'], | |||
pug: ['pug-loader'], | |||
coffee: ['babel-loader', 'coffee-loader'], |
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.
Why is the 'babel-loader' needed to make this work?
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.
babel-loader
also checks all the require
statements and adds them to the dependency graph. So this is required to make import statements work properly.
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.
Oh, wow. I'm glad I asked! :-)
@SaraVieira, @CompuIves - I think this is ready for a review. I can launch a new |
I see that some checks in |
Should address #1384 - CoffeeScript not working |
It's also failing on master :( Don't worry, that is on us |
@SaraVieira - Ok, cool |
@SaraVieira, @CompuIves - If this PR looks good and gets merged, how long does it usually take before this version of the repo is deployed on the live codesandbox.io site? |
Hey! Our deploys are ad-hock so I would say one or two days max |
Great!
…On Wed, Jan 16, 2019 at 2:07 PM Sara Vieira ***@***.***> wrote:
Hey!
Our deploys are ad-hock so I would say one or two days max
—
You are receiving this because you modified the open/close state.
Reply to this email directly, view it on GitHub
<#1467 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAIuGzNJJwvb5DeYNEem9cP6D2G77XBbks5vD5SXgaJpZM4Z_wAh>
.
|
This looks really good! Exactly the way I would implement it as well. I'm going to do some testing today and merge it in if it works properly! |
At the end of all of this, could someone summarize what to do as a normal user to get CoffeeScript working on |
@GeoffreyBooth: CoffeeScript working soon on CodeSandbox done by @shreeve |
Congrats! Thanks @shreeve! |
I made a quick screencast of how this works with the vue-cli template: |
Here's another showing how to support the parcel template: |
main.coffeeimport Vue from "vue"
import App from "./App"
Vue.config.productionTip = false
new Vue
el: "#app"
render: (h) -> h App App.vue<template lang="pug">
.example
h3 Important Stuff
p Maybe we should start with some {{ active }}...
p How about:
ul: li(v-for='item in this[active]') {{ item.join(', by ') }}
button(@click='surprise') Surprise Me!
</template>
<script lang="coffee">
export default
data: ->
active: 'jokes'
genres:
jokes: 'funny jokes'
poems: 'beautiful poems'
thoughts: 'deep thoughts'
jokes: [
[ 'How many foos does it take', 'my son Sailor' ]
[ 'Knock, knock...', 'my friend Batman' ]
[ 'So, this guy walks into a bar', 'your crazy neighbor' ] ]
poems: [
[ 'The Gods of the Copybook Headings', 'Rudyard Kipling' ]
[ 'Daffodils', 'William Wordsworth' ]
[ 'Fire and Ice', 'Robert Frost' ] ]
thoughts: [
[ 'Time is relative', 'Albert Einstein' ]
[ 'The quintic is unsolveable', 'Evariste Galois' ]
[ 'Frankly my darling...', 'Rhett Butler' ] ]
methods:
pick: (ary) ->
ary[Math.floor(Math.random() * ary.length)]
surprise: ->
list = Object.keys @genres
item = @active
item = @pick(list) until item isnt @active
@active = item
</script>
<style lang="stylus">
.example
font-family: Avenir
</style> |
Thanks @shreeve. This is amazing! Thanks for all the digging and work! |
@CompuIves / @SaraVieira - I'm not sure if there is some kind of CI check that needs to be fixed prior to this being merged, or if that top CircleCI issue is "no big deal". Is this PR able to merged or not quite yet? Thanks! |
Great! Tested it and everything works! Merging in now. |
This is a first attempt at adding CoffeeScript support to the vue-cli template for Codesandbox.
@SaraVieira recommended the Parcel template first, but I couldn't get it working. I decided to try the vue-cli template, and made some progress. This is based off the work done by @zephraph on #443, but this version is not quite yet working.
I'm submitting here to see if someone with more familiarity with Codesandbox can help make whatever adjustments for CoffeeScript support.
What kind of change does this PR introduce?
Feature; adds support for CoffeeScript in vue single file components and elsewhere.
What is the current behavior?
Addresses #1384.
What is the new behavior?
.coffee
files will be supported generally and in<script lang="coffee">
sections of.vue
files.Checklist: