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

Examples: Convert cameras and geometries to ES6. #21589

Merged
merged 2 commits into from
Apr 6, 2021
Merged

Conversation

Mugen87
Copy link
Collaborator

@Mugen87 Mugen87 commented Apr 6, 2021

Related issue: -

Description

More usage of ES6 syntax.

@Mugen87
Copy link
Collaborator Author

Mugen87 commented Apr 6, 2021

@marcofugaro When executing the conversion script on my branch, this pops up:

/Users/Michael/Documents/workspace/three.js/examples/js/exporters/USDZExporter.js
  2:1  error  Parsing error: 'import' and 'export' may only appear at the top level

/Users/Michael/Documents/workspace/three.js/examples/js/loaders/3MFLoader.js
  2:1  error  Parsing error: 'import' and 'export' may only appear at the top level

/Users/Michael/Documents/workspace/three.js/examples/js/loaders/AMFLoader.js
  2:1  error  Parsing error: 'import' and 'export' may only appear at the top level

/Users/Michael/Documents/workspace/three.js/examples/js/loaders/EXRLoader.js
  2:1  error  Parsing error: 'import' and 'export' may only appear at the top level

/Users/Michael/Documents/workspace/three.js/examples/js/loaders/FBXLoader.js
  2:1  error  Parsing error: 'import' and 'export' may only appear at the top level

/Users/Michael/Documents/workspace/three.js/examples/js/loaders/KMZLoader.js
  2:1  error  Parsing error: 'import' and 'export' may only appear at the top level

/Users/Michael/Documents/workspace/three.js/examples/js/loaders/NRRDLoader.js
  2:1  error  Parsing error: 'import' and 'export' may only appear at the top level

/Users/Michael/Documents/workspace/three.js/examples/js/loaders/TiltLoader.js
  2:1  error  Parsing error: 'import' and 'export' may only appear at the top level

/Users/Michael/Documents/workspace/three.js/examples/js/loaders/VTKLoader.js
  2:1  error  Parsing error: 'import' and 'export' may only appear at the top level

It seems the script misses to remove the following import statement. At least on my computer:

import * as fflate from '../libs/fflate.module.min.js';

@marcofugaro
Copy link
Contributor

marcofugaro commented Apr 6, 2021

@Mugen87 you're on windows right?

It is probably the \n not matching in these replaces, could you test that it doesn't happen if you remove the \n?

code = code.replace( 'import * as fflate from \'../libs/fflate.module.min.js\';\n', '' );
code = code.replace( 'import { MMDParser } from \'../libs/mmdparser.module.js\';\n', '' );
code = code.replace( 'import { potpack } from \'../libs/potpack.module.js\';\n', '' );
code = code.replace( 'import { opentype } from \'../libs/opentype.module.min.js\';\n', '' );
code = code.replace( 'import { chevrotain } from \'../libs/chevrotain.module.min.js\';\n', '' );
code = code.replace( 'import { ZSTDDecoder } from \'../libs/zstddec.module.js\';\n', '' );

@marcofugaro
Copy link
Contributor

marcofugaro commented Apr 6, 2021

The \n can be safely removed, there is a trimStart() call after. I'll make a PR if you confirm the source of the problem.

@mrdoob mrdoob added this to the r128 milestone Apr 6, 2021
@Mugen87
Copy link
Collaborator Author

Mugen87 commented Apr 6, 2021

you're on windows right?

I'm on macOS right now.

@marcofugaro
Copy link
Contributor

marcofugaro commented Apr 6, 2021

@Mugen87 tested your branch and the issue doesn't happen on my machine.

Could you help me debug this particular line on your machine? 🙏
Why this particular replace doesn't work?

code = code.replace( 'import * as fflate from \'../libs/fflate.module.min.js\';\n', '' );

@Mugen87
Copy link
Collaborator Author

Mugen87 commented Apr 6, 2021

The strange thing is import * as fflate from '../libs/fflate.module.min.js'; gets replaced with:

import { unzipSync } from '../libs/fflate.module.min.js';

And then the linter complains. Not sure why this happens yet.

@Mugen87
Copy link
Collaborator Author

Mugen87 commented Apr 6, 2021

I've removed my node_modules folder and made a fresh npm i. Now it seems to work!

@marcofugaro
Copy link
Contributor

@Mugen87 it's probably rollup, could you run a npm install to make sure you have the correct version?

I have this version:

image

@Mugen87
Copy link
Collaborator Author

Mugen87 commented Apr 6, 2021

Yep, using this version solved the issue.

@mrdoob mrdoob merged commit 2ea5cc9 into mrdoob:dev Apr 6, 2021
@mrdoob
Copy link
Owner

mrdoob commented Apr 6, 2021

Thanks!

@DefinitelyMaybe DefinitelyMaybe mentioned this pull request Apr 7, 2021
43 tasks
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 this pull request may close these issues.

3 participants