Rename JS library files from library_foo.js to libfoo.js #23348
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Marking as draft for now. |
sbc100
force-pushed
the
library_rename
branch
3 times, most recently
from
64743d3 to
49a1a75
Compare
sbc100
changed the title
kripken
reviewed
Jan 17, 2025
kripken
reviewed
Jan 17, 2025
sbc100
force-pushed
the
library_rename
branch
from
49a1a75 to
31a7e36
Compare
sbc100
force-pushed
the
library_rename
branch
4 times, most recently
from
5a3327c to
cf00d64
Compare
sbc100
added a commit
that referenced
this pull request
Jan 17, 2025
Files references via #include can now either relative to the including file or come from the system library. Previously only relative paths were allowed. Also we now give a nicer error message when the file is not found. Split out from #23348
sbc100
force-pushed
the
library_rename
branch
from
cf00d64 to
a24778e
Compare
|
Much cleaner PR now that #23449 landed |
kripken
approved these changes
Jan 17, 2025
At the same time, move all the library file into a separate `src/lib` directory. This helps distinguish library files from other files in the `lib` directory, which is currently very overcrowded/overloaded. These files were also includ-able via `-lfoo` and we previously had to use a special case for system libraries where `library_foo.js` could be used in addition to more standed `libfoo.js`. This change removes to special case. This only file that was not renamed using the simple pattern was `library.py` itself which becomes `libcore.py`. I also considered `libemscripten.js`. To some this may seem like needless churn, but for years now the `src` directory (IMHO) has been asking for cleanup. I think having files logically separated into directories helps a lot with understanding the project structure for both newcomers and old timers alike. As a followup, I am also considering moving all the runtime files into `src/include` (since these are all files that get `#included` into the generated code directly. That would leave the `src` directory containing only the JS compiler code itself which I think is another important distinction.
sbc100
force-pushed
the
library_rename
branch
from
a24778e to
e839ead
Compare
|
Added a ChangeLog entry |
|
lgtm |
sbc100
added a commit
to sbc100/emscripten
that referenced
this pull request
Feb 7, 2025
This was broken when I move the library files in emscripten-core#23348 This was only used for generating the symbol lists, so the only side effect was the the symbol lists would remain stale if somebody was modifying the system libraries (i.e. it only effected emscripten developers, not users).
sbc100
added a commit
that referenced
this pull request
Feb 7, 2025
This was broken when I move the library files in #23348 This was only used for generating the symbol lists, so the only side effect was the the symbol lists would remain stale if somebody was modifying the system libraries (i.e. it only effected emscripten developers, not users).
This was referenced Feb 19, 2025
sbc100
pushed a commit
that referenced
this pull request
Jun 14, 2025
This fixes an issue where `path.resolve()` resolves using the CWD (which is usually the target project path) instead of the directory of the script itself. The current behavior [causes issues with the Closure compiler](godotengine/godot#107460 (comment)), as stubs are erroneously created instead of being skipped in `src/shell.js`. Closure then complains that variables are declared twice. The issue was caused by #23348 which changed the logic of the search.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
At the same time, move all the library file into a separate
src/libdirectory. This helps distinguish library files from other files
in the
libdirectory, which is currently very overcrowded/overloaded.These files were also includ-able via
-lfooand we previously had touse a special case for system libraries where
library_foo.jscould beused in addition to more standed
libfoo.js. This change removes tospecial case.
This only file that was not renamed using the simple pattern was
library.pyitself which becomeslibcore.py. I also consideredlibemscripten.js.To some this may seem like needless churn, but for years now the
srcdirectory (IMHO) has been asking for cleanup. I think having files
logically separated into directories helps a lot with understanding the
project structure for both newcomers and old timers alike.
As a followup, I am also considering moving all the runtime files into
src/include(since these are all files that get#includedinto thegenerated code directly. That would leave the
srcdirectorycontaining only the JS compiler code itself which I think is another
important distinction.