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

convert all require() statements to import statements #410

Closed
zepumph opened this issue Nov 14, 2024 · 7 comments
Closed

convert all require() statements to import statements #410

zepumph opened this issue Nov 14, 2024 · 7 comments

Comments

@zepumph
Copy link
Member

zepumph commented Nov 14, 2024

While working on #403, it became clear that a large amount of overhead to convert one file to typescript will be converting each spot where that module is used to an import statement, or use require(...).default. This is less than ideal. So we would like to experiment with converting all require statements over to imports.

This also boosts typescript support since require() often provides the import as an "any" type.

@zepumph zepumph self-assigned this Nov 14, 2024
zepumph added a commit that referenced this issue Nov 15, 2024
zepumph added a commit that referenced this issue Nov 15, 2024
@zepumph
Copy link
Member Author

zepumph commented Nov 15, 2024

I got to a commit point, but I don't want to push at 5pm. I will discuss with @samreid tomorrow morning.

Next steps:

  • Push my local commit from chipper/
  • Apply same pattern to perennial
  • Address TODO about telling people to convert their js files to ts since they use imports. Not ideal.
  • Continue with Convert execute.js to typescript #403

@zepumph
Copy link
Member Author

zepumph commented Nov 15, 2024

  • Get rid of usages of __dirname in perennial using the pattern from chipper.

zepumph added a commit to phetsims/function-builder-basics that referenced this issue Nov 18, 2024
zepumph added a commit to phetsims/friction that referenced this issue Nov 18, 2024
zepumph added a commit to phetsims/fractions-intro that referenced this issue Nov 18, 2024
zepumph added a commit to phetsims/gas-properties that referenced this issue Nov 18, 2024
zepumph added a commit to phetsims/fractions-mixed-numbers that referenced this issue Nov 18, 2024
zepumph added a commit to phetsims/gene-expression-essentials that referenced this issue Nov 18, 2024
zepumph added a commit to phetsims/function-builder that referenced this issue Nov 18, 2024
zepumph added a commit to phetsims/beers-law-lab that referenced this issue Nov 18, 2024
zepumph added a commit to phetsims/area-model-decimals that referenced this issue Nov 18, 2024
zepumph added a commit to phetsims/alpenglow that referenced this issue Nov 18, 2024
zepumph added a commit to phetsims/area-builder that referenced this issue Nov 18, 2024
zepumph added a commit to phetsims/acid-base-solutions that referenced this issue Nov 18, 2024
zepumph added a commit to phetsims/area-model-algebra that referenced this issue Nov 18, 2024
zepumph added a commit to phetsims/aqua that referenced this issue Nov 18, 2024
zepumph added a commit to phetsims/axon that referenced this issue Nov 18, 2024
zepumph added a commit to phetsims/area-model-introduction that referenced this issue Nov 18, 2024
zepumph added a commit to phetsims/area-model-common that referenced this issue Nov 18, 2024
zepumph added a commit to phetsims/assert that referenced this issue Nov 18, 2024
@zepumph
Copy link
Member Author

zepumph commented Nov 19, 2024

zepumph added a commit to phetsims/chipper that referenced this issue Nov 19, 2024
zepumph added a commit to phetsims/chipper that referenced this issue Nov 19, 2024
zepumph added a commit that referenced this issue Nov 19, 2024
(cherry picked from commit f4fa761)
@zepumph
Copy link
Member Author

zepumph commented Nov 19, 2024

We opened this issue when we thought that converting to es modules may be a stepping stone to convert to typescript. We now see that actually converting to TS and running through tsx is a stepping stone to es modules. ES modules are very strict, and setting type:module in a package JSON will hard error on any usage of require, module.exports, and __dirname. We are not going to devote time to this just now, but in the future, after more Typescript coverage, this would be an excellent thing to get working (like for #396).

For now we should close this, and in the future, when the time is right, an "ES Module full conversion" issue should be opened and accomplished.

A couple more TODOs and checkboxes for refactoring is all that is left here.

@zepumph
Copy link
Member Author

zepumph commented Nov 19, 2024

Some notes, just in case:



problems:
* js file with mixed require/imports
* js file with require .js suffix for .ts file
* 


solutions:
.default <------
convert every freaking single require() and __dirname and __filename

Axioms:
- js files cannot use import statements without problems
- we do not want to set type:module in all package.jsons
- tsx auto detects when no require() statements are around, and will error out on __dirname
- We cannot use `type: module` with a gruntfile in the repo.



To convert to a typescript file:
All usages of require( myModule) need a `.default` at the end.




TODO: 
- [x] search for convert `import * from fs` to `import fs`, and other modules too?
- [x] type errors
- [x] lint errors
- [x] note the revert in perennial-alias
- [x] make an issue for converting all require statements to import statements.
- [x] }; EOF check
- [x] chipper tests, change chains to bumper.

general troubles of importing a ts file from a js file

- [ ] Search for Gruntfile.js to see where we need to support .cjs
- [ ] localInfo.js rename to .cjs
- [x] Search for `require.*chipper/js/` to make sure none need `.default` or conversion to ts.
- [x] handle all chipper entrypoints that are currently .js.
- [x] extractDocumentation

-> perennial/js/common doesn't need to change eagerly.
	why?
		because `tsx myScript.ts` which imports a js file is ok to have module.exports and require();

ONLY ISSUE is .js + even one import statement + tsx launching




What repos use module.exports not in a grunt file?
aqua
binder\js
chipper
perennial-alias
perennial
phet-info\sim-info
phet-lib\js
phet-vite-demo
quake\cordova-plugin-native-vibration\src\browser
quake\cordova-plugin-native-vibration\www
query-string-machine\js
rosetta\scripts
weddell
yotta\js
	^- only localeInfo from chipper



Hello! Isoscelemur team has pushed to basically all repos. This change converted most of chipper/ to typescript, updated pre commit hooks for maintainability, and has better support for commonjs vs module-type javascript execution. 
Please do the following:
* pull all
* re install git hooks with `perennial-alias/bin/for-each.sh perennial-alias/data/active-repos "rm .git/hooks/pre-commit; git init --template=../phet-info/git-template-dir"`
* I expect at this point most/all attempts to use `node` to run javascript scripts to fail. This is a friendly reminder to update your environment and practices to instead use `sage run`.
* Report any trouble to our team. We will be on the look out for expected trouble on CT(Q).

zepumph added a commit to phetsims/dot that referenced this issue Nov 19, 2024
zepumph added a commit to phetsims/chipper that referenced this issue Nov 19, 2024
zepumph added a commit to phetsims/chipper that referenced this issue Nov 19, 2024
@zepumph
Copy link
Member Author

zepumph commented Nov 19, 2024

All has been handled here. Closing

@zepumph zepumph closed this as completed Nov 19, 2024
zepumph added a commit to phetsims/aqua that referenced this issue Nov 19, 2024
zepumph added a commit to phetsims/chipper that referenced this issue Nov 19, 2024
zepumph added a commit that referenced this issue Nov 19, 2024
zepumph added a commit that referenced this issue Nov 19, 2024
zepumph added a commit to phetsims/phet-info that referenced this issue Nov 19, 2024
zepumph added a commit that referenced this issue Nov 20, 2024
This reverts commit a4c5bfc.
samreid pushed a commit to phetsims/chipper that referenced this issue Nov 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant