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

Update Traceur to the latest version #79

Open
adrian-seijo opened this issue Jan 22, 2015 · 11 comments
Open

Update Traceur to the latest version #79

adrian-seijo opened this issue Jan 22, 2015 · 11 comments

Comments

@adrian-seijo
Copy link

Hi,

If I'm not mistaken es6ify is currently using Traceur 0.0.79 and there is a newer version available, 0.0.81. That one seems to solve some weird issues we have in our project so it would be awesome if we can have es6ify updated to that version.

Cheers!

@domenic
Copy link
Collaborator

domenic commented Jan 22, 2015

A pull request would be welcome. Otherwise I probably should get around to #45 so everyone can bring their own Traceur.

@GirlBossRush
Copy link

+1

@jmm
Copy link
Collaborator

jmm commented Jan 29, 2015

I didn't even think there was an 0.0.81 -- there's no tag for it. At any rate, 0.0.82 is out now. This seems to address an issue I'm having where local paths that I don't want exposed are included in the output as sourceURL values.

@domenic That would consist of bumping the version in package.json and dealing with test failures?

@domenic
Copy link
Collaborator

domenic commented Jan 29, 2015

@jmm yep. IIRC the way Traceur handles options has changed pretty drastically.

@jmm
Copy link
Collaborator

jmm commented Jan 29, 2015

@domenic Ok, thanks. It would be really nice if there was a changelog for traceur.

@jmm
Copy link
Collaborator

jmm commented Jan 30, 2015

@domenic I started working on this but quickly got into the weeds. Just doing this currently fails with node v0.10.32 (linux) / traceur v0.0.82:

var
  vm = require('vm'),
  fs = require('fs'),
  traceur = require('traceur');

vm.runInNewContext(
  fs.readFileSync(traceur.RUNTIME_PATH)
);

Result:

vm.runInNewContext(
   ^
ModuleEvaluationError: Float32Array is not defined
    at evalmachine.<anonymous>:2780:17

See nodejs/node-v0.x-archive/issues/9121.

So that's interfering with some of the tests (e.g. test/bundle.js).

@jmm
Copy link
Collaborator

jmm commented Jan 30, 2015

Ok, I worked around the Float32Array issue per the suggestion in nodejs/node-v0.x-archive#9121. There is now one remaining test failure, that I'll be looking into.

@jmm
Copy link
Collaborator

jmm commented Jan 30, 2015

@domenic In test/transform.js, what exactly is the "transform adds sourcemap comment and uses cache on second time" test supposed to be testing? Is it really valuable to do a deepEqual() on the whole source map? Isn't the sourcesContent data for the code generated by traceur prone to change by nature (as it's an implementation detail)? With traceur 0.0.82 the test is failing partly because traceur is inserting an additional generated file in the source map and because of a different name for a generated file in the sources array (e.g. @traceur/generated/TemplateParser/1 !== @traceur/generated/TemplateParser/2). I don't want to go digging into this nitty-gritty kind of traceur stuff, like figuring out what the deal is with these generated files in the source map, unless it's necessary.

@domenic
Copy link
Collaborator

domenic commented Jan 30, 2015

@thlorenz can probably give a better idea on what the procedure is for those kind of tests. I agree they are fragile and brittle. However they have caught several regressions too :-/.

@domenic
Copy link
Collaborator

domenic commented Jan 30, 2015

Maybe a start would be: I think it's probably fine if you rebaseline them to the latest results. In the past the things they've caught have been e.g. empty source maps or source maps with wrong paths.

@jmm
Copy link
Collaborator

jmm commented Jan 30, 2015

Maybe a start would be: I think it's probably fine if you rebaseline them to the latest results.

Ok, I'll take a look at that. I'd like to hear what @thlorenz has to say about the necessity of testing to that specificity. I created a work in progress PR: /pull/80

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants