-
Notifications
You must be signed in to change notification settings - Fork 248
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
Breaking: v6 with backward compatibility #3370
base: master
Are you sure you want to change the base?
Conversation
const isProduction = false; | ||
// These will be overridden/added to in the package.json of any plugin | ||
// they are here for backward compatibility only | ||
const v5toV6DefaultMappings = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
with the maps and paths now specified here can we remove the corresponding config from scriptLoader?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No. The ones in the scriptLoader.js
are client-side and they happen before adapt.min.js
is loaded, the mappings from the compiler happen at the top of adapt.min.js
, after those modules are needed. Oftentimes external libraries/
files will expect jQuery at minimum and will need access to jQuery before adapt.min.js
is loaded.
We will be able to remove some of them as we move over to node_modules, specifically: regenerator-runtime
, core-js
, react
, react-dom
, html-react-parser
and semver
. The others will be more difficult.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah, of course, though is the scriptLoader map used at all?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The only use I saw was spoor's offline_API_wrapper.js, but I think that has recently been moved into compilation, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The map
properties are used heavilly in adapt-devtools
. The paths
section is used further down the scriptLoader.js
to load the foundational libraries+templates and some of those libraries are used by name at various places throughout the core and plugins.
map
coreJS: 'core/js',
coreViews: 'core/js/views',
coreModels: 'core/js/models',
coreCollections: 'core/js/collections'
These shortcuts were mostly removed when we switched to babel and jsdoc, both of which really only work correctly with resolvable file paths.
#930
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Effectively, when core is updated to switch to node_modules
, we can also drop the scriptLoader.js
directives, where reasonable to do so, considering backward compatibility and load order. jQuery
, jqueryMobile
, underscore
, backbone
, handlebars
and Modernizer
will be most difficult I imagine. imageReady
, inview
and jquery.resize
are libraries I made, so they should be quite easy to integrate into the core. bowser
, scrollTo
, requirejs
and velocity
I haven't analysed yet.
fixes #2526
fixes #2363
original issues #3044
original prototype #3324
What's changed?
src/course
in favour ofbuild/course
Testing
ctrl+c
, then runTodo
adapt install
command etc will be made v6 compatible Add npm layer adapt-cli#175includes
do not yet work with dependent extensions, such that if resources is included but drawer is not, drawer will not yet be added to the construction phase