Skip to content

es compatibility #19

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

Closed
atiertant opened this issue Oct 20, 2016 · 18 comments
Closed

es compatibility #19

atiertant opened this issue Oct 20, 2016 · 18 comments

Comments

@atiertant
Copy link

hi, could you replace src/utils.js Line 25

  • ' (require as any).ensure([], function (require: any) {',
  • ' require.ensure([], function () {',

to be compatible with javascript? (don't know if it work with typescript)

@brandonroberts
Copy link
Owner

This will break TypeScript because there will be missing support for require.ensure conflicting with node typings. I could add an option to the loader for a vanilla javascript version of the load string.

@brandonroberts
Copy link
Owner

If you're using Webpack 2, you can use the loader=system option

@atiertant
Copy link
Author

@brandonroberts i use webpack 1 ...

@brandonroberts
Copy link
Owner

It still works with Webpack 1 if you're using TypeScript. Will you provide a reproduction in a gist or repo?

@atiertant
Copy link
Author

@brandonroberts i wouldn't like to support an es version, i would prefer contribute to your...
i forked just to add my repo in my package.json for now, if you wouldn't like support es compatibility, i'll push it on npm.

@brandonroberts
Copy link
Owner

I would like to support your issue as well. My question is are you using TypeScript or not?

@atiertant
Copy link
Author

no, i'm using javascript

@brandonroberts
Copy link
Owner

I haven't forgotten about your issue. Would it be sufficient to check the file extension in order to determine whether to add the type? If the extension is ts, use require as any. If its js, just use require. What do you think?

@atiertant
Copy link
Author

@brandonroberts looks a good idea

@ayyash
Copy link

ayyash commented Dec 23, 2016

I used loader=system, it generates the right line but in production I get the error EXCEPTION: Uncaught (in promise): ReferenceError: System is not defined
I am not sure if default loader would produce this error (haven't got this far), the main setup I have is that I use systemjs throughout development, and I only webpack everything from the js files before production, so I do not reference the ts transpiler in webpack at all, the tsc I use already produced the required js files. So, that being said, the output cannot have "typescript" or "es6" specific statements... is that even possible in this case?

@vepasto
Copy link

vepasto commented Mar 22, 2017

@brandonroberts @atiertant Check my pull request. It adds ES support by query parameter. What you think?

@atiertant
Copy link
Author

@vepasto couldn't we use @brandonroberts solution?
with something like var es = filePath.endsWith('.js'); in getRequireLoader function ?

@vepasto
Copy link

vepasto commented Mar 23, 2017

@atiertant is it reliable enough since TS project can contain .js files?

@atiertant
Copy link
Author

@vepasto what kind of .js file in a TS project can contain a loadChildren ? see microsoft/TypeScript#2302

@vepasto
Copy link

vepasto commented Mar 24, 2017

@atiertant I'm not sure. To be honest I'm not very familiar with TS. It just sounded dangerous as we're making potentially backwards incompatible changes, and someone's build could possibly break. That's why I suggested the safe way.

If you suggest it is safe, can you make a pull request?

@vepasto
Copy link

vepasto commented Mar 28, 2017

Is anyone going to make that pull request? @atiertant @brandonroberts

@atiertant
Copy link
Author

@vepasto will not have time for now, @brandonroberts do you think #66 could be merged ?

@brandonroberts
Copy link
Owner

Support for plain JS files released as 0.6.0

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

Successfully merging a pull request may close this issue.

4 participants